[PATCH] Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan

[PATCH] Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan

[Pauli Virtanen]

l2cap_new_connection() sets default value of channel mode to match the
parent channel.  l2cap_le_connect_req() left this at the default, and
created L2CAP_MODE_EXT_FLOWCTL channels if listening pchan has that
mode.  This causes FLAG_DEFER_SETUP channels to reply to
L2CAP_LE_CONN_REQ with L2CAP_ECRED_CONN_RSP, which is incorrect.

It can also result to stack OOB write (of l2cap_alloc_cid determined
values) in l2cap_ecred_rsp_defer(), as l2cap_le_connect_req() does not
limit maximum number of deferred channels or check for duplicate ident.

Fix by setting chan->mode correctly in l2cap_le_connect_req().

Also check channel mode in l2cap_ecred_rsp_defer(), and do WARN_ON_ONCE
instead of OOB write to make it less brittle.

Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    This is another pre-existing issue from sashiko.dev reviews

 net/bluetooth/l2cap_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 358b11eabd4f..fa7dbf5f448e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3886,6 +3886,9 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
 	struct l2cap_ecred_conn_rsp *rsp_flex =
 		container_of(&rsp->pdu.rsp, struct l2cap_ecred_conn_rsp, hdr);
 
+	if (chan->mode != L2CAP_MODE_EXT_FLOWCTL)
+		return;
+
 	/* Check if channel for outgoing connection or if it wasn't deferred
 	 * since in those cases it must be skipped.
 	 */
@@ -3896,6 +3899,10 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
 	/* Reset ident so only one response is sent */
 	chan->ident = 0;
 
+	/* Unreachable, check in l2cap_ecred_conn_req. If reached, drop rest */
+	if (WARN_ON_ONCE(rsp->count >= ARRAY_SIZE(rsp->pdu.scid)))
+		rsp->pdu.rsp.result = cpu_to_le16(L2CAP_CR_LE_NO_MEM);
+
 	/* Include all channels pending with the same ident */
 	if (!rsp->pdu.rsp.result)
 		rsp_flex->dcid[rsp->count++] = cpu_to_le16(chan->scid);
@@ -5064,6 +5071,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
 	__set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
 
 	chan->ident = cmd->ident;
+	chan->mode = L2CAP_MODE_LE_FLOWCTL;
 
 	if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
 		l2cap_state_change(chan, BT_CONNECT2);
-- 
2.55.0

RE: Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      0.56 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.21 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      27.19 seconds
CheckAllWarning               PASS      29.96 seconds
CheckSparse                   PASS      28.45 seconds
BuildKernel32                 PASS      26.03 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      496.64 seconds
TestRunner_l2cap-tester       PASS      63.92 seconds
IncrementalBuild              PASS      24.93 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


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

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan

[Pauli Virtanen]

su, 2026-08-30 kello 20:11 +0300, Pauli Virtanen kirjoitti:
> l2cap_new_connection() sets default value of channel mode to match the
> parent channel.  l2cap_le_connect_req() left this at the default, and
> created L2CAP_MODE_EXT_FLOWCTL channels if listening pchan has that
> mode.  This causes FLAG_DEFER_SETUP channels to reply to
> L2CAP_LE_CONN_REQ with L2CAP_ECRED_CONN_RSP, which is incorrect.
> 
> It can also result to stack OOB write (of l2cap_alloc_cid determined
> values) in l2cap_ecred_rsp_defer(), as l2cap_le_connect_req() does not
> limit maximum number of deferred channels or check for duplicate ident.
> 
> Fix by setting chan->mode correctly in l2cap_le_connect_req().
> 
> Also check channel mode in l2cap_ecred_rsp_defer(), and do WARN_ON_ONCE
> instead of OOB write to make it less brittle.

On Sashiko review comments:

> > +	/* Unreachable, check in l2cap_ecred_conn_req. If reached, drop rest */
> > +	if (WARN_ON_ONCE(rsp->count >= ARRAY_SIZE(rsp->pdu.scid)))
> > +		rsp->pdu.rsp.result = cpu_to_le16(L2CAP_CR_LE_NO_MEM);
> > +
> >  	/* Include all channels pending with the same ident */
> >  	if (!rsp->pdu.rsp.result)
> >  		rsp_flex->dcid[rsp->count++] = cpu_to_le16(chan->scid);
> >  	else
> >  		l2cap_chan_del(chan, ECONNRESET);
> 
> Can this lead to a self-deadlock and list corruption?
> If the WARN_ON_ONCE condition is met, the result is set to L2CAP_CR_LE_NO_MEM,
> causing the code to fall through to the else branch and call l2cap_chan_del().
> When an L2CAP LE socket is created with BT_SK_DEFER_SETUP, the deferred
> setup path calls l2cap_sock_recvmsg() which acquires the socket lock, and
> then it calls __l2cap_ecred_conn_rsp_defer() without acquiring conn->lock.
> Calling l2cap_chan_del() here unconditionally calls list_del() on the
> unprotected channel list, which corrupts the list. Furthermore, it invokes
> the teardown callback, which attempts to acquire the socket lock again,
> resulting in a self-deadlock since l2cap_sock_recvmsg() already holds it.

This is about the l2cap_chan_del() call there which indeed can
deadlock, but it is pre-existing issue. The WARN_ON should also be
unreachable.

The pre-existing deadlock is reachable on bluetooth-next/master, as in
this test:

https://lore.kernel.org/linux-bluetooth/3d6b440c7c3a077daf91a47bbc578a2cd40f2a01.1788119838.git.pav@iki.fi/

The locking is fixed in

https://lore.kernel.org/linux-bluetooth/3213ea451b4db947b3333c002ab19678431781eb.1788013041.git.pav@iki.fi/

> >  	chan->ident = cmd->ident;
> > +	chan->mode = L2CAP_MODE_LE_FLOWCTL;
> >  
> >  	if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
> >  		l2cap_state_change(chan, BT_CONNECT2);
> 
> This isn't a bug introduced by this patch, but does locklessly assigning
> fields like chan->mode here create a data race?
> In l2cap_le_connect_req(), l2cap_new_connection() is called earlier, which
> invokes l2cap_sock_new_connection_cb() and immediately enqueues the socket
> into the parent's accept queue. This exposes the socket to userspace before
> initialization is complete.
> If userspace accepts the socket and calls setsockopt(), could it race with
> these lockless initializations, allowing userspace to observe or corrupt
> the uninitialized socket state? This patch exacerbates the issue by adding
> another unprotected assignment.

These data races should be cleaned up later, probably adding
__guarded_by() annotations at the same time.

> Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
> 
> Notes:
>     This is another pre-existing issue from sashiko.dev reviews
> 
>  net/bluetooth/l2cap_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 358b11eabd4f..fa7dbf5f448e 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3886,6 +3886,9 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
>  	struct l2cap_ecred_conn_rsp *rsp_flex =
>  		container_of(&rsp->pdu.rsp, struct l2cap_ecred_conn_rsp, hdr);
>  
> +	if (chan->mode != L2CAP_MODE_EXT_FLOWCTL)
> +		return;
> +
>  	/* Check if channel for outgoing connection or if it wasn't deferred
>  	 * since in those cases it must be skipped.
>  	 */
> @@ -3896,6 +3899,10 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
>  	/* Reset ident so only one response is sent */
>  	chan->ident = 0;
>  
> +	/* Unreachable, check in l2cap_ecred_conn_req. If reached, drop rest */
> +	if (WARN_ON_ONCE(rsp->count >= ARRAY_SIZE(rsp->pdu.scid)))
> +		rsp->pdu.rsp.result = cpu_to_le16(L2CAP_CR_LE_NO_MEM);
> +
>  	/* Include all channels pending with the same ident */
>  	if (!rsp->pdu.rsp.result)
>  		rsp_flex->dcid[rsp->count++] = cpu_to_le16(chan->scid);
> @@ -5064,6 +5071,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
>  	__set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
>  
>  	chan->ident = cmd->ident;
> +	chan->mode = L2CAP_MODE_LE_FLOWCTL;
>  
>  	if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
>  		l2cap_state_change(chan, BT_CONNECT2);

-- 
Pauli Virtanen

Re: [PATCH] Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Sun, 30 Aug 2026 20:11:36 +0300 you wrote:
> l2cap_new_connection() sets default value of channel mode to match the
> parent channel.  l2cap_le_connect_req() left this at the default, and
> created L2CAP_MODE_EXT_FLOWCTL channels if listening pchan has that
> mode.  This causes FLAG_DEFER_SETUP channels to reply to
> L2CAP_LE_CONN_REQ with L2CAP_ECRED_CONN_RSP, which is incorrect.
> 
> It can also result to stack OOB write (of l2cap_alloc_cid determined
> values) in l2cap_ecred_rsp_defer(), as l2cap_le_connect_req() does not
> limit maximum number of deferred channels or check for duplicate ident.
> 
> [...]

Here is the summary with links:
  - Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan
    https://git.kernel.org/bluetooth/bluetooth-next/c/b1f1766ef769

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html