All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen
@ 2026-04-28 23:30 Safa Karakuş
  2026-04-29  0:37 ` bluez.test.bot
  2026-04-30 17:17 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 5+ messages in thread
From: Safa Karakuş @ 2026-04-28 23:30 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org
  Cc: marcel@holtmann.org, luiz.dentz@gmail.com, stable@vger.kernel.org

l2cap_sock_cleanup_listen() dequeues child sockets via
bt_accept_dequeue() without holding a reference on the returned sk.
A concurrent HCI disconnect can trigger l2cap_conn_del() on CPU1
which, while holding chan->lock, calls:

  teardown_cb  -> sock_set_flag(sk, SOCK_ZAPPED)
  close_cb     -> l2cap_sock_kill(sk) -> sock_put(sk) -> kfree(sk)

all before CPU0 has a chance to acquire chan->lock.  CPU0 then calls
l2cap_chan_lock() on the now-freed sk's chan (already safe because
l2cap_chan_hold() was called first) but subsequently passes the freed
sk pointer to l2cap_sock_kill(), causing a use-after-free read on
sk->sk_flags and sk->sk_socket.

Fix by calling sock_hold() immediately after bt_accept_dequeue() to
prevent kfree(sk) from racing with our traversal.  After acquiring
chan->lock, check SOCK_DEAD: if l2cap_conn_del() already invoked
l2cap_sock_kill() (which sets SOCK_DEAD), skip the duplicate call to
avoid a double sock_put().  Drop the extra reference with sock_put()
at the end of each loop iteration.

Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: stable@vger.kernel.org
Signed-off-by: Safa Karakus<safa.karakus@secunnix.com>
---
 net/bluetooth/l2cap_sock.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 71e8c1b45..4475d3377 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1477,7 +1477,15 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)

 	/* Close not yet accepted channels */
 	while ((sk = bt_accept_dequeue(parent, NULL))) {
-		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+		struct l2cap_chan *chan;
+
+		/* Hold sk across the chan->lock acquisition window.
+		 * A concurrent l2cap_conn_del() can call l2cap_sock_kill(sk)
+		 * -> kfree(sk) inside chan->lock before we acquire it,
+		 * leaving a dangling pointer.
+		 */
+		sock_hold(sk);
+		chan = l2cap_pi(sk)->chan;

 		BT_DBG("child chan %p state %s", chan,
 		       state_to_string(chan->state));
@@ -1487,10 +1495,16 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)

 		__clear_chan_timer(chan);
 		l2cap_chan_close(chan, ECONNRESET);
-		l2cap_sock_kill(sk);
+		/* l2cap_conn_del() may have already called l2cap_sock_kill()
+		 * (setting SOCK_DEAD); skip the duplicate to avoid a
+		 * double sock_put().
+		 */
+		if (!sock_flag(sk, SOCK_DEAD))
+			l2cap_sock_kill(sk);

 		l2cap_chan_unlock(chan);
 		l2cap_chan_put(chan);
+		sock_put(sk);
 	}
 }

--
2.34.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen
  2026-04-28 23:30 [PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen Safa Karakuş
@ 2026-04-29  0:37 ` bluez.test.bot
  2026-04-30 17:17 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2026-04-29  0:37 UTC (permalink / raw)
  To: linux-bluetooth, safa.karakus

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1087159

---Test result---

Test Summary:
CheckPatch                    FAIL      0.68 seconds
GitLint                       PASS      0.29 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      26.11 seconds
CheckAllWarning               PASS      28.61 seconds
CheckSparse                   PASS      27.17 seconds
BuildKernel32                 PASS      24.89 seconds
TestRunnerSetup               PASS      564.19 seconds
TestRunner_l2cap-tester       PASS      27.43 seconds
IncrementalBuild              PASS      24.06 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen
WARNING: From:/Signed-off-by: email name mismatch: 'From: Safa Karakuş <safa.karakus@secunnix.com>' != 'Signed-off-by: Safa Karakus<safa.karakus@secunnix.com>'

total: 0 errors, 1 warnings, 0 checks, 33 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14545605.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


Wide character in print at /github/workspace/src/src/scripts/checkpatch.pl line 7853.


https://github.com/bluez/bluetooth-next/pull/129

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen
  2026-04-28 23:30 [PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen Safa Karakuş
  2026-04-29  0:37 ` bluez.test.bot
@ 2026-04-30 17:17 ` Luiz Augusto von Dentz
  2026-05-16  9:21   ` [PATCH v3] Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del() Safa Karakuş
  1 sibling, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2026-04-30 17:17 UTC (permalink / raw)
  To: Safa Karakuş
  Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
	stable@vger.kernel.org

Hi Safa,

On Tue, Apr 28, 2026 at 7:30 PM Safa Karakuş <safa.karakus@secunnix.com> wrote:
>
> l2cap_sock_cleanup_listen() dequeues child sockets via
> bt_accept_dequeue() without holding a reference on the returned sk.
> A concurrent HCI disconnect can trigger l2cap_conn_del() on CPU1
> which, while holding chan->lock, calls:
>
>   teardown_cb  -> sock_set_flag(sk, SOCK_ZAPPED)
>   close_cb     -> l2cap_sock_kill(sk) -> sock_put(sk) -> kfree(sk)
>
> all before CPU0 has a chance to acquire chan->lock.  CPU0 then calls
> l2cap_chan_lock() on the now-freed sk's chan (already safe because
> l2cap_chan_hold() was called first) but subsequently passes the freed
> sk pointer to l2cap_sock_kill(), causing a use-after-free read on
> sk->sk_flags and sk->sk_socket.
>
> Fix by calling sock_hold() immediately after bt_accept_dequeue() to
> prevent kfree(sk) from racing with our traversal.  After acquiring
> chan->lock, check SOCK_DEAD: if l2cap_conn_del() already invoked
> l2cap_sock_kill() (which sets SOCK_DEAD), skip the duplicate call to
> avoid a double sock_put().  Drop the extra reference with sock_put()
> at the end of each loop iteration.
>
> Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
> Cc: stable@vger.kernel.org
> Signed-off-by: Safa Karakus<safa.karakus@secunnix.com>
> ---
>  net/bluetooth/l2cap_sock.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 71e8c1b45..4475d3377 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1477,7 +1477,15 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
>
>         /* Close not yet accepted channels */
>         while ((sk = bt_accept_dequeue(parent, NULL))) {
> -               struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> +               struct l2cap_chan *chan;
> +
> +               /* Hold sk across the chan->lock acquisition window.
> +                * A concurrent l2cap_conn_del() can call l2cap_sock_kill(sk)
> +                * -> kfree(sk) inside chan->lock before we acquire it,
> +                * leaving a dangling pointer.
> +                */
> +               sock_hold(sk);
> +               chan = l2cap_pi(sk)->chan;
>
>                 BT_DBG("child chan %p state %s", chan,
>                        state_to_string(chan->state));
> @@ -1487,10 +1495,16 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
>
>                 __clear_chan_timer(chan);
>                 l2cap_chan_close(chan, ECONNRESET);
> -               l2cap_sock_kill(sk);
> +               /* l2cap_conn_del() may have already called l2cap_sock_kill()
> +                * (setting SOCK_DEAD); skip the duplicate to avoid a
> +                * double sock_put().
> +                */
> +               if (!sock_flag(sk, SOCK_DEAD))
> +                       l2cap_sock_kill(sk);
>
>                 l2cap_chan_unlock(chan);
>                 l2cap_chan_put(chan);
> +               sock_put(sk);
>         }
>  }
>
> --
> 2.34.1

sashiko flags 2 critical flaws with these changes:

https://sashiko.dev/#/patchset/AS8P250MB079109F82C16BEDC4F9FE584EB372%40AS8P250MB0791.EURP250.PROD.OUTLOOK.COM

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3] Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del()
  2026-04-30 17:17 ` [PATCH] " Luiz Augusto von Dentz
@ 2026-05-16  9:21   ` Safa Karakuş
  2026-05-16 10:18     ` [v3] " bluez.test.bot
  0 siblings, 1 reply; 5+ messages in thread
From: Safa Karakuş @ 2026-05-16  9:21 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Luiz Augusto von Dentz, Marcel Holtmann, stable, linux-kernel,
	Safa Karakuş

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

CVE-2025-39860 only serialised the two userspace threads racing
bt_accept_dequeue() (cleanup_listen() under lock_sock() in
l2cap_sock_release()); it does not cover l2cap_conn_del() from
hci_rx_work, so this still reproduces on v7.0-rc5.

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: 12 use-after-free per run before, 0 and no lockdep over
1400+ raced iterations after.

Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: stable@vger.kernel.org
Signed-off-by: Safa Karakuş <safa.karakus@secunnix.com>
---
Hi Luiz,

v3 - addresses the sashiko findings on v1.  An interim approach using
conn->lock closed the UAF but hit a lockdep inversion, so this pins the
chan via a brief child sk lock instead.

Changes since v2:
 - Take the ref inside bt_accept_dequeue() (v1/v2 added sock_hold()
   after it, racing the free); also fix the chan lifetime; no
   conn->lock (lockdep).  Reproduced on v7.0-rc5 post CVE-2025-39860:
   12 UAF/run -> 0.
Changes since v1: consistent From/Signed-off-by.

 net/bluetooth/af_bluetooth.c | 10 +++++++
 net/bluetooth/iso.c          |  9 ++++++-
 net/bluetooth/l2cap_sock.c   | 51 +++++++++++++++++++++++++++++++-----
 net/bluetooth/rfcomm/sock.c  |  9 ++++++-
 net/bluetooth/sco.c          |  9 ++++++-
 5 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 2b94e2077..10eafe7c1 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -309,6 +309,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);
 			return sk;
 		}
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index be145e273..94732563d 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -759,6 +759,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 */
@@ -1364,8 +1366,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;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 71e8c1b45..61f2b20a7 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -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_sock_kill(sk);
-
+		/* 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);
 	}
 }
 
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index be6639cd6..bd7d959c6 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -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;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 584e059de..72bcbf1da 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -487,6 +487,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;
@@ -743,8 +745,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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [v3] Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del()
  2026-05-16  9:21   ` [PATCH v3] Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del() Safa Karakuş
@ 2026-05-16 10:18     ` bluez.test.bot
  0 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2026-05-16 10:18 UTC (permalink / raw)
  To: linux-bluetooth, safa.karakus

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: net/bluetooth/af_bluetooth.c:309
error: net/bluetooth/af_bluetooth.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-16 10:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 23:30 [PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen Safa Karakuş
2026-04-29  0:37 ` bluez.test.bot
2026-04-30 17:17 ` [PATCH] " Luiz Augusto von Dentz
2026-05-16  9:21   ` [PATCH v3] Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del() Safa Karakuş
2026-05-16 10:18     ` [v3] " bluez.test.bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.