[PATCH] Bluetooth: L2CAP: reject accept queue add unless BT_LISTEN

[Pauli Virtanen]

New items should not be added to parent socket accept queue after last
l2cap_sock_cleanup_listen() has run in l2cap_sock_teardown_cb() and
state set to BT_CLOSED, as that can result to UAF on dereferencing the
dangling parent reference.

Add check for sk_state == BT_LISTEN after acquiring sk lock in
l2cap_sock_new_connection_cb() to avoid this.

Add lock_sock() around sk_state changes where it was missing.

That this can trigger appears to be possible since l2cap_chan::state is
not consistently guarded by a lock, so data races on it can exist in
checking parent pchan->state == BT_LISTEN in l2cap_core.

Fixes: 2ff1a41a912d ("Bluetooth: L2CAP: Fix null-ptr-deref in l2cap_sock_state_change_cb()")
Reported-by: syzbot+9265e7...@syzkaller.appspotmail.com
---

#syz test

net/bluetooth/l2cap_sock.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 735167f73f31..8c2ac8b911e0 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1568,6 +1568,11 @@ static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan,

lock_sock(parent);

+ if (parent->sk_state != BT_LISTEN) {
+ release_sock(parent);
+ return -EINVAL;
+ }
+
/* Check for backlog size */
if (sk_acceptq_is_full(parent)) {
BT_DBG("backlog full %d", parent->sk_ack_backlog);
@@ -1731,10 +1736,14 @@ static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
if (!sk)
return;

+ lock_sock(sk);
+
sk->sk_state = state;

if (err)
sk->sk_err = err;
+
+ release_sock(sk);
}

static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
@@ -1810,6 +1819,8 @@ static void l2cap_sock_resume_cb(struct l2cap_chan *chan)
if (!sk)
return;

+ lock_sock(sk);
+
if (test_and_clear_bit(FLAG_PENDING_SECURITY, &chan->flags)) {
sk->sk_state = BT_CONNECTED;
chan->state = BT_CONNECTED;
@@ -1817,6 +1828,8 @@ static void l2cap_sock_resume_cb(struct l2cap_chan *chan)

clear_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags);
sk->sk_state_change(sk);
+
+ release_sock(sk);
}

static void l2cap_sock_set_shutdown_cb(struct l2cap_chan *chan)
--
2.55.0

[syzbot]

Hello,

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

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

Tested on:

commit: a7c7074b Merge tag 'fbdev-for-7.2-rc7' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17660149580000
kernel config: https://syzkaller.appspot.com/x/.config?x=4887c52b0f7d06a5
dashboard link: https://syzkaller.appspot.com/bug?extid=9265e754091c2d27ea29
compiler: Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
patch: https://syzkaller.appspot.com/x/patch.diff?x=158fefb9580000

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

[Pauli Virtanen]

New sk should not be added to parent socket accept queue after last

l2cap_sock_cleanup_listen() has run in l2cap_sock_teardown_cb() and
state set to BT_CLOSED, as that can result to UAF on dereferencing the
dangling parent reference.

l2cap_sock_new_connection_cb() may race with parent l2cap_chan teardown,
due to chan->state accessed without consistent locking:

[Task 1] [Task 2]
l2cap_sock_release(parent) l2cap_connect
l2cap_sock_shutdown pchan = l2cap_global_chan_by_psm
l2cap_chan_lock(pchan)
l2cap_chan_close
l2cap_sock_teardown_cb
pchan->state = BT_CLOSED
l2cap_chan_unlock(pchan) ------> l2cap_chan_lock(pchan)
l2cap_new_connection
l2cap_sock_new_connection_cb
l2cap_chan_lock(pchan) <-------- l2cap_chan_unlock(pchan)
l2cap_sock_kill(parent) /* bt_sk(sk)->parent dangling */

Fix by adding check for sk_state == BT_LISTEN after acquiring sk lock in
l2cap_sock_new_connection_cb(). Add lock_sock() around sk_state writes
where missing, to avoid data races.

Although the data races on pchan->state should be fixed too, this
defensive sk_state check probably makes sense in any case.

Fixes: 2ff1a41a912d ("Bluetooth: L2CAP: Fix null-ptr-deref in l2cap_sock_state_change_cb()")
Reported-by: syzbot+9265e7...@syzkaller.appspotmail.com

Closes: https://syzkaller.appspot.com/bug?extid=9265e754091c2d27ea29
Signed-off-by: Pauli Virtanen <p...@iki.fi>
---

Notes:
Fixing up chan->state locking is a bigger changeset, and should be done
separately.

[syzbot]

Hello,

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

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

Tested on:

commit: a7c7074b Merge tag 'fbdev-for-7.2-rc7' of git://git.ke..
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=1105bfb9580000

kernel config: https://syzkaller.appspot.com/x/.config?x=4887c52b0f7d06a5
dashboard link: https://syzkaller.appspot.com/bug?extid=9265e754091c2d27ea29
compiler: Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8

patch: https://syzkaller.appspot.com/x/patch.diff?x=10176132580000