[PATCH v2 1/2] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect

[PATCH v2 1/2] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect

[Pauli Virtanen]

l2cap_chan_connect() tries to ensure there are no more than
L2CAP_ECRED_CONN_SCID_MAX pending ECRED channels, so they fit in the
same L2CAP_ECRED_CONN_REQ that l2cap_ecred_connect() constructs.

However, the check only counts deferred channels.  If 6 L2CAP sockets
are connected at the same time in order DDDDND (D=deferred,
N=non-deferred), the last can bump the total to max+1.  It results to
one __le16 written out of bounds of the scid array, and an invalid
ECRED_CONN_REQ being sent.

Fix by leaving room for the non-deferred pending ECRED channels in the
counting in l2cap_chan_connect(), so the limit can't be exceeded.

Move counting under same critical section where the channel is added.
Although race conditions involving this appear unreachable, it's easier
to see.

Also add WARN_ON_ONCE check in l2cap_ecred_defer_connect() to make this
less brittle.

Fixes: da49b602f7f7 ("Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    Bug found as pre-existing in sashiko.dev report
    
    v2:
    - Do counting differently, to avoid calling chan->ops->get_peer_pid for
      non-deferred channels.
    - Move counting inside existing conn->lock critical section, it's better
      for locking context of get_peer_pid.

 net/bluetooth/l2cap_core.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 358b11eabd4f..1b612b9beaa8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1329,7 +1329,7 @@ static void l2cap_le_connect(struct l2cap_chan *chan)
 struct l2cap_ecred_conn_data {
 	struct {
 		struct l2cap_ecred_conn_req_hdr req;
-		__le16 scid[5];
+		__le16 scid[L2CAP_ECRED_CONN_SCID_MAX];
 	} __packed pdu;
 	struct l2cap_chan *chan;
 	struct pid *pid;
@@ -1357,6 +1357,10 @@ static void l2cap_ecred_defer_connect(struct l2cap_chan *chan, void *data)
 	if (test_and_set_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
 		return;
 
+	/* Unreachable, checked in l2cap_connect (+timer drops it if reached) */
+	if (WARN_ON_ONCE(conn->count >= ARRAY_SIZE(conn->pdu.scid)))
+		return;
+
 	l2cap_ecred_init(chan, 0);
 
 	/* Set the same ident so we can match on the rsp */
@@ -7374,6 +7378,9 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		goto done;
 	}
 
+	mutex_lock(&conn->lock);
+	l2cap_chan_lock(chan);
+
 	if (chan->mode == L2CAP_MODE_EXT_FLOWCTL) {
 		struct l2cap_chan_data data;
 
@@ -7381,19 +7388,20 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		data.pid = chan->ops->get_peer_pid(chan);
 		data.count = 1;
 
-		l2cap_chan_list(conn, l2cap_chan_by_pid, &data);
+		__l2cap_chan_list(conn, l2cap_chan_by_pid, &data);
+
+		/* Leave room for non-deferred channel that ends the group. */
+		if (test_bit(FLAG_DEFER_SETUP, &chan->flags))
+			data.count += 1;
 
 		/* Check if there isn't too many channels being connected */
 		if (data.count > L2CAP_ECRED_CONN_SCID_MAX) {
 			hci_conn_drop(hcon);
 			err = -EPROTO;
-			goto done;
+			goto chan_unlock;
 		}
 	}
 
-	mutex_lock(&conn->lock);
-	l2cap_chan_lock(chan);
-
 	if (cid && __l2cap_get_chan_by_dcid(conn, cid)) {
 		hci_conn_drop(hcon);
 		err = -EBUSY;
-- 
2.55.0

[PATCH v2 2/2] Bluetooth: L2CAP: clear FLAG_DEFER_SETUP only for same PID/PSM

[Pauli Virtanen]

l2cap_ecred_defer_connect() clears FLAG_DEFER_SETUP also for channels
with different PID/PSM, which will not be added to the same
ECRED_CONN_REQ in any case. Consequently, only one ECRED connection
group can work at a time although it appears intended they would be
separate for each PID/PSM combination.

Fix by clearing FLAG_DEFER_SETUP only for the connections that could be
added in the request. Retain test_bit(FLAG_DEFER_SETUP) before calling
get_peer_pid as it may be NULL otherwise.

Fixes: da49b602f7f7 ("Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    Another sashiko.dev pre-existing issue
    
    v2:
    - new commit in series

 net/bluetooth/l2cap_core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1b612b9beaa8..e3d955d7d4fa 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1344,7 +1344,7 @@ static void l2cap_ecred_defer_connect(struct l2cap_chan *chan, void *data)
 	if (chan == conn->chan)
 		return;
 
-	if (!test_and_clear_bit(FLAG_DEFER_SETUP, &chan->flags))
+	if (!test_bit(FLAG_DEFER_SETUP, &chan->flags))
 		return;
 
 	pid = chan->ops->get_peer_pid(chan);
@@ -1354,6 +1354,9 @@ static void l2cap_ecred_defer_connect(struct l2cap_chan *chan, void *data)
 	    chan->mode != L2CAP_MODE_EXT_FLOWCTL || chan->state != BT_CONNECT)
 		return;
 
+	if (!test_and_clear_bit(FLAG_DEFER_SETUP, &chan->flags))
+		return;
+
 	if (test_and_set_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
 		return;
 
-- 
2.55.0

RE: [v2,1/2] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      1.96 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       FAIL      0.67 seconds
SubjectPrefix                 PASS      0.26 seconds
BuildKernel                   PASS      27.91 seconds
CheckAllWarning               PASS      30.37 seconds
CheckSparse                   PASS      29.43 seconds
BuildKernel32                 PASS      26.83 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      507.47 seconds
TestRunner_l2cap-tester       PASS      64.99 seconds
IncrementalBuild              PASS      29.26 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2,1/2] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect

28: B2 Line has trailing whitespace: "    "
[v2,2/2] Bluetooth: L2CAP: clear FLAG_DEFER_SETUP only for same PID/PSM

18: B2 Line has trailing whitespace: "    "
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


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

---
Regards,
Linux Bluetooth

Re: [PATCH v2 1/2] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect

[patchwork-bot+bluetooth]

Hello:

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

On Sun, 30 Aug 2026 15:04:01 +0300 you wrote:
> l2cap_chan_connect() tries to ensure there are no more than
> L2CAP_ECRED_CONN_SCID_MAX pending ECRED channels, so they fit in the
> same L2CAP_ECRED_CONN_REQ that l2cap_ecred_connect() constructs.
> 
> However, the check only counts deferred channels.  If 6 L2CAP sockets
> are connected at the same time in order DDDDND (D=deferred,
> N=non-deferred), the last can bump the total to max+1.  It results to
> one __le16 written out of bounds of the scid array, and an invalid
> ECRED_CONN_REQ being sent.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect
    https://git.kernel.org/bluetooth/bluetooth-next/c/ddaccd985bb0
  - [v2,2/2] Bluetooth: L2CAP: clear FLAG_DEFER_SETUP only for same PID/PSM
    https://git.kernel.org/bluetooth/bluetooth-next/c/af04b0e3176e

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