[PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen

[PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen

[Safa Karakuş]

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

RE: Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen

[bluez.test.bot]

[-- 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

Re: [PATCH] Bluetooth: l2cap: fix UAF race in l2cap_sock_cleanup_listen

[Luiz Augusto von Dentz]

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