Bluetooth: fix UAF in bt_accept_dequeue()

bt_accept_get() takes a temporary reference before dropping the accept
queue lock. bt_accept_dequeue() currently drops that reference before
bt_accept_unlink(), leaving only the queue reference.

bt_accept_unlink() drops the queue reference. The subsequent
sock_hold() therefore accesses freed memory if it was the final
reference, as observed by KASAN during listening L2CAP socket cleanup.

Retain the temporary queue-walk reference through unlink and hand it to
the caller on success. Drop it explicitly on the closed and
not-yet-connected paths.

Fixes: ab1513597c ("Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del()")
Reported-by: syzbot+674ff7e4d7fdfd572afc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=674ff7e4d7fdfd572afc
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
Yousef Alhouseen 2026-06-28 02:23:05 +02:00 committed by Luiz Augusto von Dentz
parent bb067a99a0
commit 4bd0b27405
2 changed files with 5 additions and 16 deletions

View File

@ -305,7 +305,7 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
restart:
for (sk = bt_accept_get(parent, NULL); sk; sk = next) {
/* Prevent early freeing of sk due to unlink and sock_kill */
/* The reference from bt_accept_get() keeps sk alive. */
lock_sock(sk);
/* Check sk has not already been unlinked via
@ -321,13 +321,11 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
next = bt_accept_get(parent, sk);
/* sk is safely in the parent list so reduce reference count */
sock_put(sk);
/* FIXME: Is this check still needed */
if (sk->sk_state == BT_CLOSED) {
bt_accept_unlink(sk);
release_sock(sk);
sock_put(sk);
continue;
}
@ -337,16 +335,6 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
if (newsock)
sock_graft(sk, newsock);
/* Hand the caller a reference taken while sk is
* still locked. bt_accept_unlink() just dropped
* the accept-queue reference; without this hold a
* concurrent teardown (e.g. l2cap_conn_del() ->
* l2cap_sock_kill()) could free sk between
* release_sock() and the caller using it. Every
* caller drops this with sock_put() when done.
*/
sock_hold(sk);
release_sock(sk);
if (next)
sock_put(next);
@ -354,6 +342,7 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
}
release_sock(sk);
sock_put(sk);
}
return NULL;

View File

@ -1492,8 +1492,8 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
/* Close not yet accepted channels.
*
* bt_accept_dequeue() now returns sk with an extra reference held
* (taken while sk was still locked) so a concurrent l2cap_conn_del()
* 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.
*
* cleanup_listen() runs under the parent sk lock, so unlike