mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-08-31 11:41:29 -04:00
Code Issues Packages Projects Releases Wiki Activity

Bluetooth: L2CAP: fix race l2cap_sock_cleanup_listen() vs. put_chan

Browse Source 
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: 6fef032af0 ("Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()")
Reported-by: syzbot+e6382a2f53f5fc7453ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e6382a2f53f5fc7453ac
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
Pauli Virtanen
2026-08-08 12:08:45 +03:00
committed by Luiz Augusto von Dentz
parent 59eecbe2f2
commit 66d6ef1854
2 changed files with 18 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
include/net/bluetooth/l2cap.h
View File

@@ -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;
};

23
net/bluetooth/l2cap_sock.c
View File

@@ -1344,7 +1344,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);
@@ -1548,14 +1553,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
@@ -1565,10 +1566,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;
}