Commit ab151359 authored by Safa Karakuş's avatar Safa Karakuş Committed by Luiz Augusto von Dentz
Browse files

Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del() 



bt_accept_dequeue() unlinks a not-yet-accepted child from the parent
accept queue and release_sock()s it before returning, so the returned
sk has no caller reference and is unlocked.

l2cap_sock_cleanup_listen() walks these children on listening-socket
close.  A concurrent HCI disconnect drives hci_rx_work ->
l2cap_conn_del() which runs l2cap_chan_del() + l2cap_sock_kill() and
frees the child sk and its l2cap_chan; cleanup_listen() then uses both:

  BUG: KASAN: slab-use-after-free in l2cap_sock_kill
    l2cap_sock_kill / l2cap_sock_cleanup_listen / __x64_sys_close
  Freed by: l2cap_conn_del -> l2cap_sock_close_cb -> l2cap_sock_kill

This is distinct from the two fixes already in this area: commit
e83f5e24 ("Bluetooth: serialize accept_q access") serialises the
accept_q list/poll and takes temporary refs inside bt_accept_dequeue(),
and CVE-2025-39860 serialises the userspace close()/accept() race by
calling cleanup_listen() under lock_sock() in l2cap_sock_release().
Neither covers l2cap_conn_del() running from hci_rx_work, so this UAF
still reproduces on current bluetooth/master.

Take the reference at the source: bt_accept_dequeue() does sock_hold()
while sk is still locked, before release_sock(); callers sock_put().
cleanup_listen() pins the chan with l2cap_chan_hold_unless_zero() under
a brief child sk lock (serialising vs l2cap_sock_teardown_cb()), drops
it before l2cap_chan_lock(), and skips a duplicate l2cap_sock_kill() on
SOCK_DEAD.  conn->lock is not taken here: cleanup_listen() runs under
the parent sk lock and that would invert
conn->lock -> chan->lock -> sk_lock (lockdep).

KASAN/SMP: an unprivileged listen/close vs HCI-disconnect race produced
12 use-after-free reports per run before this change; 0, and no lockdep
report, over 1600+ raced iterations after it on bluetooth/master.

Fixes: 15f02b91 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: stable@vger.kernel.org
Reported-by: default avatarSiwei Zhang <oss@fourdim.xyz>
Reviewed-by: default avatarSiwei Zhang <oss@fourdim.xyz>
Signed-off-by: default avatarSafa Karakuş <safa.karakus@secunnix.com>
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent c1bb9336

net/bluetooth/af_bluetooth.c

+10 −0
Original line number Diff line number Diff line
@@ -340,6 +340,16 @@ 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);

net/bluetooth/iso.c

+8 −1
Original line number Diff line number Diff line
@@ -751,6 +751,8 @@ static void iso_sock_cleanup_listen(struct sock *parent) 
	while ((sk = bt_accept_dequeue(parent, NULL))) {

		iso_sock_close(sk);

		iso_sock_kill(sk);

		/* Drop the reference handed back by bt_accept_dequeue(). */

		sock_put(sk);

	}



	/* If listening socket has a hcon, properly disconnect it */
@@ -1356,8 +1358,13 @@ static int iso_sock_accept(struct socket *sock, struct socket *newsock, 
		}



		ch = bt_accept_dequeue(sk, newsock);

		if (ch)

		if (ch) {

			/* Drop the bridging ref from bt_accept_dequeue();

			 * the grafted socket keeps ch alive from here.

			 */

			sock_put(ch);

			break;

		}



		if (!timeo) {

			err = -EAGAIN;

net/bluetooth/l2cap_sock.c

+44 −7
Original line number Diff line number Diff line
@@ -349,8 +349,13 @@ static int l2cap_sock_accept(struct socket *sock, struct socket *newsock, 
		}



		nsk = bt_accept_dequeue(sk, newsock);

		if (nsk)

		if (nsk) {

			/* Drop the bridging ref from bt_accept_dequeue();

			 * the grafted socket keeps nsk alive from here.

			 */

			sock_put(nsk);

			break;

		}



		if (!timeo) {

			err = -EAGAIN;
@@ -1475,22 +1480,54 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) 
	BT_DBG("parent %p state %s", parent,

	       state_to_string(parent->sk_state));



	/* Close not yet accepted channels */

	/* 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()

	 * -> l2cap_sock_kill() cannot free sk under us.

	 *

	 * cleanup_listen() runs under the parent sk lock, so unlike

	 * l2cap_sock_shutdown() we must NOT take conn->lock here: that would

	 * 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.

	 */

	while ((sk = bt_accept_dequeue(parent, NULL))) {

		struct l2cap_chan *chan = l2cap_pi(sk)->chan;

		struct l2cap_chan *chan;



		lock_sock_nested(sk, L2CAP_NESTING_NORMAL);

		chan = l2cap_chan_hold_unless_zero(l2cap_pi(sk)->chan);

		release_sock(sk);

		if (!chan) {

			/* l2cap_conn_del() already tearing this child down */

			sock_put(sk);

			continue;

		}



		BT_DBG("child chan %p state %s", chan,

		       state_to_string(chan->state));



		l2cap_chan_hold(chan);

		l2cap_chan_lock(chan);



		__clear_chan_timer(chan);

		l2cap_chan_close(chan, ECONNRESET);

		/* l2cap_conn_del() may already have killed this socket

		 * (it sets SOCK_DEAD); skip the duplicate to avoid a

		 * double sock_put()/l2cap_chan_put().

		 */

		if (!sock_flag(sk, SOCK_DEAD))

			l2cap_sock_kill(sk);



		l2cap_chan_unlock(chan);



		l2cap_chan_put(chan);

		sock_put(sk);

	}

}

net/bluetooth/rfcomm/sock.c

+8 −1
Original line number Diff line number Diff line
@@ -180,6 +180,8 @@ static void rfcomm_sock_cleanup_listen(struct sock *parent) 
	while ((sk = bt_accept_dequeue(parent, NULL))) {

		rfcomm_sock_close(sk);

		rfcomm_sock_kill(sk);

		/* Drop the reference handed back by bt_accept_dequeue(). */

		sock_put(sk);

	}



	parent->sk_state  = BT_CLOSED;
@@ -497,8 +499,13 @@ static int rfcomm_sock_accept(struct socket *sock, struct socket *newsock, 
		}



		nsk = bt_accept_dequeue(sk, newsock);

		if (nsk)

		if (nsk) {

			/* Drop the bridging ref from bt_accept_dequeue();

			 * the grafted socket keeps nsk alive from here.

			 */

			sock_put(nsk);

			break;

		}



		if (!timeo) {

			err = -EAGAIN;

net/bluetooth/sco.c

+8 −1
Original line number Diff line number Diff line
@@ -502,6 +502,8 @@ static void sco_sock_cleanup_listen(struct sock *parent) 
	while ((sk = bt_accept_dequeue(parent, NULL))) {

		sco_sock_close(sk);

		sco_sock_kill(sk);

		/* Drop the reference handed back by bt_accept_dequeue(). */

		sock_put(sk);

	}



	parent->sk_state  = BT_CLOSED;
@@ -765,8 +767,13 @@ static int sco_sock_accept(struct socket *sock, struct socket *newsock, 
		}



		ch = bt_accept_dequeue(sk, newsock);

		if (ch)

		if (ch) {

			/* Drop the bridging ref from bt_accept_dequeue();

			 * the grafted socket keeps ch alive from here.

			 */

			sock_put(ch);

			break;

		}



		if (!timeo) {

			err = -EAGAIN;