[PATCH] Bluetooth: L2CAP: fix race l2cap_sock_cleanup_listen() vs. put_chan

[Pauli Virtanen]

For L2CAP sockets without owning sk->sk_socket, reading
l2cap_pi(sk)->chan may race against concurrent l2cap_sock_kill() ->
l2cap_sock_put_chan(). This excludes simultaneous proto_ops callbacks,
but access in l2cap_sock_cleanup_listen() has unsafe lockless read.

Fix the race by taking lock_sock() in l2cap_sock_kill() to
synchronize with l2cap_sock_cleanup_listen(). hold_unless_zero() is not
needed here, l2cap_pi(sk)->chan owns reference if it is non-NULL.

Fixes: 0e2c0392b9dc ("Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()")
Reported-by: syzbot+e6382a...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e6382a2f53f5fc7453ac
Signed-off-by: Pauli Virtanen <p...@iki.fi>
---
include/net/bluetooth/l2cap.h | 5 +++++
net/bluetooth/l2cap_sock.c | 23 +++++++++++++----------
2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index ef6ce1c20a4f..3d9a32094347 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -699,7 +699,12 @@ struct l2cap_rx_busy {

struct l2cap_pinfo {
struct bt_sock bt;
+
+ /* With owning sk_socket chan may be read without lock, other access
+ * should hold lock_sock.
+ */
struct l2cap_chan *chan;
+
struct list_head rx_busy;
};

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 735167f73f31..9540617a0e6c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1312,7 +1312,12 @@ static void l2cap_sock_kill(struct sock *sk)

BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));

+ /* Take lock to synchronize against access without owning sk->sk_socket,
+ * eg. in l2cap_sock_cleanup_listen(). proto_ops etc. don't need lock.
+ */
+ lock_sock(sk);
l2cap_sock_put_chan(sk);
+ release_sock(sk);

/* Kill poor orphan */
sock_set_flag(sk, SOCK_DEAD);
@@ -1516,14 +1521,10 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
* establish sk_lock -> conn->lock and invert the established
* conn->lock -> chan->lock -> sk_lock order (lockdep deadlock).
*
- * Instead, briefly take the child sk lock to fetch and pin its chan.
- * l2cap_conn_del() reaches the chan free only via
- * l2cap_chan_del() -> l2cap_sock_teardown_cb(), which itself takes
- * the child sk lock; holding it across l2cap_chan_hold_unless_zero()
- * therefore guarantees the chan cannot be freed while we read and
- * pin it (hold_unless_zero() additionally skips a chan already past
- * its last reference). We then drop the sk lock before taking
- * chan->lock, so sk and chan locks are never held together.
+ * Instead, briefly take the child sk lock to synchronize vs.
+ * l2cap_sock_kill that puts l2cap_pi(sk)->chan. We then drop the sk
+ * lock before taking chan->lock, so sk and chan locks are never held
+ * together.
*
* Since we cannot call l2cap_chan_close() without conn->lock,
* schedule l2cap_chan_timeout to close the channel; it already
@@ -1533,10 +1534,12 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
struct l2cap_chan *chan;

lock_sock_nested(sk, L2CAP_NESTING_NORMAL);
- chan = l2cap_chan_hold_unless_zero(l2cap_pi(sk)->chan);
+ chan = l2cap_pi(sk)->chan;
+ if (chan)
+ l2cap_chan_hold(chan);
release_sock(sk);
if (!chan) {
- /* l2cap_conn_del() already tearing this child down */
+ /* Already torn down */
sock_put(sk);
continue;
}
--
2.55.0

[Hillf Danton]

In l2cap_sock_teardown_cb(), sock is only zapped after cleanup including unlink,
so why do you see a linked and zapped sock in l2cap_sock_cleanup_listen()?

[Pauli Virtanen]

Hi,

ma, 2026-08-03 kello 14:13 +0800, Hillf Danton kirjoitti:
> On Sun, 2 Aug 2026 15:12:28 +0300 Pauli Virtanen wrote:

> In l2cap_sock_teardown_cb(), sock is only zapped after cleanup including unlink,
> so why do you see a linked and zapped sock in l2cap_sock_cleanup_listen()?

l2cap_sock_cleanup_listen() is not a single critical section.

There is the following race:

[Task 1] [Task 2 (hdev->workqueue)]
l2cap_sock_release(parent) l2cap_disconn_cfm
l2cap_sock_cleanup_listen l2cap_conn_del
bt_accept_dequeue l2cap_chan_del
lock_sock(sk) l2cap_sock_teardown_cb
bt_accept_unlink
bt_sk(sk)->parent = NULL
release_sock(sk) ----------------> lock_sock(sk)
parent = bt_sk(sk)->parent /* == NULL */
lock_sock(sk) <--------------------- release_sock(sk)
sock_set_flag(sk, SOCK_ZAPPED)
l2cap_sock_close_cb
l2cap_sock_kill(sk)
l2cap_sock_put_chan
chan = READ l2cap_pi(sk)->chan l2cap_pi(sk)->chan = NULL
l2cap_chan_hold_unless_zero l2cap_put_chan(chan)
kref_get_unless_zero(&chan->ref)

[Hillf Danton]

The race window is still open after this work.

release_sock(sk)
sock_set_flag(sk, SOCK_ZAPPED)
l2cap_sock_close_cb
l2cap_sock_kill(sk)
l2cap_sock_put_chan

l2cap_pi(sk)->chan = NULL
l2cap_put_chan(chan)
sock_set_flag(sk, SOCK_DEAD);
sock_put(sk); // free sk
lock_sock(sk) // uaf

chan = READ l2cap_pi(sk)->chan

l2cap_chan_hold_unless_zero

[Pauli Virtanen]

Hi,

There is no UAF there, Task 1 holds a reference on sk at this point, if
you look at the code sock_put() follows.

I don't think there is a remaining problem.

--
Pauli Virtanen

[Hillf Danton]

On Tue, 04 Aug 2026 08:40:16 +0300 Pauli Virtanen wrote:
>
> There is no UAF there, Task 1 holds a reference on sk at this point, if
> you look at the code sock_put() follows.
>

If the subsequent put works without both race and mm leak, then it is the
very evidence of uaf with race.

[Pauli Virtanen]

ti, 2026-08-04 kello 16:16 +0800, Hillf Danton kirjoitti:
> On Tue, 04 Aug 2026 08:40:16 +0300 Pauli Virtanen wrote:

> > There is no UAF there, Task 1 holds a reference on sk at this point, if
> > you look at the code sock_put() follows.
> >
> If the subsequent put works without both race and mm leak, then it is the
> very evidence of uaf with race.

This and the "// uaf" and "// free sk" above appear to ignore what is
explained also in the comment in l2cap_sock_cleanup_listen():

* bt_accept_dequeue() returns sk with its temporary queue-
walk
* reference held, so a concurrent l2cap_conn_del()
* -> l2cap_sock_kill() cannot free sk under us.

bt_accept_dequeue() returns a new temporary reference to sk, which is
owned by the caller who needs to put it after use.

The "sock_put(sk); // free sk" cannot free sk, because
bt_accept_dequeue(sk) got a new reference on sk before it, so the
refcount is at least 2 here. Then "lock_sock(sk) // uaf" cannot be UAF.

If chan != NULL in Task 1, it schedules l2cap_chan teardown. That
eventually ends up in l2cap_sock_kill() to put the reference owned by
l2cap_chan & sk_socket.

If chan == NULL in Task 1, l2cap_chan cleanup is completed or
concurrent, and nothing more needs to be done.

--
Pauli Virtanen

[Pauli Virtanen]

For L2CAP sockets without owning sk->sk_socket, reading
l2cap_pi(sk)->chan may race against concurrent l2cap_sock_kill() ->
l2cap_sock_put_chan(). This excludes simultaneous proto_ops callbacks,
but access in l2cap_sock_cleanup_listen() has unsafe lockless read.

[Task 1] [Task 2 (hdev->workqueue)]
l2cap_sock_release(parent) l2cap_disconn_cfm
l2cap_sock_cleanup_listen l2cap_conn_del
bt_accept_dequeue l2cap_chan_del
lock_sock(sk) l2cap_sock_teardown_cb
bt_accept_unlink
bt_sk(sk)->parent = NULL
release_sock(sk) ----------------> lock_sock(sk)

parent = /* NULL */

lock_sock(sk) <--------------------- release_sock(sk)
sock_set_flag(sk, SOCK_ZAPPED)
l2cap_sock_close_cb
l2cap_sock_kill(sk)
l2cap_sock_put_chan
chan = READ l2cap_pi(sk)->chan l2cap_pi(sk)->chan = NULL
l2cap_chan_hold_unless_zero l2cap_put_chan(chan)
kref_get_unless_zero(&chan->ref)

Task 1 may observe NULL which causes null-ptr-deref.

Fix the race by taking lock_sock() in l2cap_sock_kill() to
synchronize with l2cap_sock_cleanup_listen(). hold_unless_zero() is not
needed here, l2cap_pi(sk)->chan owns reference if it is non-NULL.

Clarify code comments vs. locking.

Fixes: 0e2c0392b9dc ("Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()")
Reported-by: syzbot+e6382a...@syzkaller.appspotmail.com

---

#syz test

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+e6382a...@syzkaller.appspotmail.com
Tested-by: syzbot+e6382a...@syzkaller.appspotmail.com

Tested on:

commit: a59f57e2 Merge tag 'watchdog-for-v7.2-rc7' of git://gi..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16f26fb9580000
kernel config: https://syzkaller.appspot.com/x/.config?x=c44651ea7dd2f307
dashboard link: https://syzkaller.appspot.com/bug?extid=e6382a2f53f5fc7453ac
compiler: gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
patch: https://syzkaller.appspot.com/x/patch.diff?x=132fefb9580000

Note: testing is done by a robot and is best-effort only.