[PATCH RESEND v4 0/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

[PATCH RESEND v4 0/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

[Siwei Zhang]

This addresses v2 comments on https://sashiko.dev/#/patchset/20260415204842.2363950-1-oss%40fourdim.xyz .

Compared to v3, rebase against bluetooth-next.

Resend due to the missing version number.

Siwei Zhang (1):
  Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

 net/bluetooth/6lowpan.c    |  5 +++++
 net/bluetooth/l2cap_core.c | 12 ++++++++++++
 net/bluetooth/l2cap_sock.c | 13 ++++++++++++-
 net/bluetooth/smp.c        |  5 +++++
 4 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.54.0

[PATCH RESEND v4 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

[Siwei Zhang]

l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
release_sock(parent). Once the parent lock is released, the child
socket sk can be freed by another task.

Save the channel pointer into a local variable while the parent lock
is still held to prevent this.

Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
Cc: stable@kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
---
 net/bluetooth/6lowpan.c    |  5 +++++
 net/bluetooth/l2cap_core.c | 12 ++++++++++++
 net/bluetooth/l2cap_sock.c | 13 ++++++++++++-
 net/bluetooth/smp.c        |  5 +++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 23a229ab6a33..71c1c04b61e5 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -755,6 +755,11 @@ static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
 
 	BT_DBG("chan %p pchan %p", chan, pchan);
 
+	/* Match the put that the caller of ops->new_connection() performs
+	 * once it is done with the returned channel pointer.
+	 */
+	l2cap_chan_hold(chan);
+
 	return chan;
 }
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7701528f1167..0f6c3c651207 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4071,6 +4071,9 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 
 	__l2cap_chan_add(conn, chan);
 
+	/* Drop the ops->new_connection() ref; conn list now pins chan. */
+	l2cap_chan_put(chan);
+
 	dcid = chan->scid;
 
 	__set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
@@ -4970,6 +4973,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
 
 	__l2cap_chan_add(conn, chan);
 
+	/* Drop the ops->new_connection() ref; conn list now pins chan. */
+	l2cap_chan_put(chan);
+
 	l2cap_le_flowctl_init(chan, __le16_to_cpu(req->credits));
 
 	dcid = chan->scid;
@@ -5194,6 +5200,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
 
 		__l2cap_chan_add(conn, chan);
 
+		/* Drop the ops->new_connection() ref; conn list now pins chan. */
+		l2cap_chan_put(chan);
+
 		l2cap_ecred_init(chan, __le16_to_cpu(req->credits));
 
 		/* Init response */
@@ -7407,6 +7416,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 			chan->dst_type = dst_type;
 
 			__l2cap_chan_add(conn, chan);
+
+			/* Drop the ops->new_connection() ref; conn list now pins chan. */
+			l2cap_chan_put(chan);
 		}
 
 		l2cap_chan_unlock(pchan);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index cf590a67d364..295c79cf5cf3 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1497,6 +1497,7 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
 static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk, *parent = chan->data;
+	struct l2cap_chan *child_chan;
 
 	if (!parent)
 		return NULL;
@@ -1523,9 +1524,19 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 
 	bt_accept_enqueue(parent, sk, false);
 
+	child_chan = l2cap_pi(sk)->chan;
+
+	/* Pin the channel for the caller. Once release_sock(parent) returns,
+	 * userspace can accept(2) and immediately close(2) the child socket,
+	 * which would drop the socket's references on the channel and free
+	 * it before the caller (e.g. l2cap_connect_req()) is done using the
+	 * returned pointer. The matching put is the caller's responsibility.
+	 */
+	l2cap_chan_hold(child_chan);
+
 	release_sock(parent);
 
-	return l2cap_pi(sk)->chan;
+	return child_chan;
 }
 
 static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 1739c1989dbd..9796c3030434 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -3231,6 +3231,11 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan)
 
 	BT_DBG("created chan %p", chan);
 
+	/* Match the put that the caller of ops->new_connection() performs
+	 * once it is done with the returned channel pointer.
+	 */
+	l2cap_chan_hold(chan);
+
 	return chan;
 }
 
-- 
2.54.0

RE: Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1708 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=1092984

---Test result---

Test Summary:
CheckPatch                    PASS      1.40 seconds
GitLint                       FAIL      0.34 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      26.78 seconds
CheckAllWarning               PASS      27.90 seconds
CheckSparse                   PASS      26.85 seconds
BuildKernel32                 PASS      24.71 seconds
TestRunnerSetup               PASS      524.97 seconds
TestRunner_l2cap-tester       PASS      376.41 seconds
TestRunner_smp-tester         PASS      18.41 seconds
TestRunner_6lowpan-tester     PASS      51.29 seconds
IncrementalBuild              PASS      23.95 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[RESEND,v4,1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (86>80): "[RESEND,v4,1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()"


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

---
Regards,
Linux Bluetooth

Re: [PATCH RESEND v4 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()

[Luiz Augusto von Dentz]

Hi,

On Mon, May 11, 2026 at 1:09 PM Siwei Zhang <oss@fourdim.xyz> wrote:
>
> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after
> release_sock(parent). Once the parent lock is released, the child
> socket sk can be freed by another task.
>
> Save the channel pointer into a local variable while the parent lock
> is still held to prevent this.
>
> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c")
> Cc: stable@kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
> ---
>  net/bluetooth/6lowpan.c    |  5 +++++
>  net/bluetooth/l2cap_core.c | 12 ++++++++++++
>  net/bluetooth/l2cap_sock.c | 13 ++++++++++++-
>  net/bluetooth/smp.c        |  5 +++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index 23a229ab6a33..71c1c04b61e5 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -755,6 +755,11 @@ static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
>
>         BT_DBG("chan %p pchan %p", chan, pchan);
>
> +       /* Match the put that the caller of ops->new_connection() performs
> +        * once it is done with the returned channel pointer.
> +        */
> +       l2cap_chan_hold(chan);
> +
>         return chan;
>  }
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7701528f1167..0f6c3c651207 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4071,6 +4071,9 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>
>         __l2cap_chan_add(conn, chan);
>
> +       /* Drop the ops->new_connection() ref; conn list now pins chan. */
> +       l2cap_chan_put(chan);
> +
>         dcid = chan->scid;
>
>         __set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
> @@ -4970,6 +4973,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
>
>         __l2cap_chan_add(conn, chan);
>
> +       /* Drop the ops->new_connection() ref; conn list now pins chan. */
> +       l2cap_chan_put(chan);
> +
>         l2cap_le_flowctl_init(chan, __le16_to_cpu(req->credits));
>
>         dcid = chan->scid;
> @@ -5194,6 +5200,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>
>                 __l2cap_chan_add(conn, chan);
>
> +               /* Drop the ops->new_connection() ref; conn list now pins chan. */
> +               l2cap_chan_put(chan);
> +
>                 l2cap_ecred_init(chan, __le16_to_cpu(req->credits));
>
>                 /* Init response */
> @@ -7407,6 +7416,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
>                         chan->dst_type = dst_type;
>
>                         __l2cap_chan_add(conn, chan);
> +
> +                       /* Drop the ops->new_connection() ref; conn list now pins chan. */
> +                       l2cap_chan_put(chan);
>                 }
>
>                 l2cap_chan_unlock(pchan);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index cf590a67d364..295c79cf5cf3 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1497,6 +1497,7 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
>  static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>  {
>         struct sock *sk, *parent = chan->data;
> +       struct l2cap_chan *child_chan;
>
>         if (!parent)
>                 return NULL;
> @@ -1523,9 +1524,19 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>
>         bt_accept_enqueue(parent, sk, false);
>
> +       child_chan = l2cap_pi(sk)->chan;
> +
> +       /* Pin the channel for the caller. Once release_sock(parent) returns,
> +        * userspace can accept(2) and immediately close(2) the child socket,
> +        * which would drop the socket's references on the channel and free
> +        * it before the caller (e.g. l2cap_connect_req()) is done using the
> +        * returned pointer. The matching put is the caller's responsibility.
> +        */
> +       l2cap_chan_hold(child_chan);

The entire problem might be solvable by not removing `list_add` from
`l2cap_create_chan`. This way, it only allocates but does not attach
to global_l until __l2cap_chan_add is called which then handles the
addition. Alternatively, we could allocate it first, given the
circular dependency involving `l2cap_core`.c->l2cap_sock.c)
new_connection -> l2cap_chan_create(l2cap_sock.c->l2cap_core.c) which
makes the code rather hard to follow.

>         release_sock(parent);
>
> -       return l2cap_pi(sk)->chan;
> +       return child_chan;
>  }
>
>  static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 1739c1989dbd..9796c3030434 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -3231,6 +3231,11 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan)
>
>         BT_DBG("created chan %p", chan);
>
> +       /* Match the put that the caller of ops->new_connection() performs
> +        * once it is done with the returned channel pointer.
> +        */
> +       l2cap_chan_hold(chan);
> +
>         return chan;
>  }
>
> --
> 2.54.0
>


-- 
Luiz Augusto von Dentz