[PATCH 0/2] Bluetooth: L2CAP: Match LE responses to outstanding requests

[PATCH 0/2] Bluetooth: L2CAP: Match LE responses to outstanding requests

[Zhao Li]

An LE peer that can reach a local L2CAP listener can use unsolicited
responses or Command Rejects to bypass deferred setup, remove an
established channel, or leak an unaccepted socket and channel.
Authentication requirements depend on the listener's security setting;
whether a default configuration exposes such a listener has not been
checked.

Patch 1 requires an outstanding request of the matching kind before a
response or reject can change a channel. It also completes socket
cleanup when a reconfigure failure or reject arrives after local close.
Patch 2 clears incoming identifiers once their responses have been
sent, so a later ECRED request that reuses one no longer matches the old
channel. Patch 1 is the security fix; patch 2 is separable.

Patch 1 extends the BR/EDR lookup from commit a518b9a9ef6e ("Bluetooth:
L2CAP: Only accept connection responses to outstanding requests"). Its
stable tags request 5.7 and newer with that prerequisite or an equivalent
backport. Older LE-only trees need an adapted backport without the ECRED
hunks.

AI assistance was used.

GCC 12.2 and Clang 19.1.7 builds passed at W=1, including full x86_64
module linking after each patch. GCC also compiled the Bluetooth code
built-in on x86_64 and as modules on i386. Sparse reported no diagnostic;
Smatch retained its one pre-existing diagnostic. Coccinelle 1.3's
use_after_iter rule found nothing in the changed functions; five
unchanged parser gaps also occur in the base.

QEMU/vhci testing with patch 1 alone kept all 445 established channels
alive. In 40-pair close-before runs, failed reconfigure responses and
Command Rejects each destroyed about 40 sockets with the fix and only
boot-noise sockets on the base. The success path destroyed the same
number on both. A 1200-pair base run retained gross Slab memory after
listener close while the fixed arms released it.

With patch 2, new ECRED requests that reused the identifier of a
closing LE channel completed instead of being refused; partially
accepted groups were not tested. A KCSAN kernel with both patches
reported no data races in the close-before runs.

Hardware validation has not been run. Patch 1 does not apply to stable
branches as is; adapted backports of both patches compiled at W=1 and
linked as modules on the seven supported lines, and were not tested.

Zhao Li (2):
  Bluetooth: L2CAP: Only accept LE responses to outstanding requests
  Bluetooth: L2CAP: Clear request identifiers on incoming LE connections

 include/net/bluetooth/l2cap.h |  1 +
 net/bluetooth/l2cap_core.c    | 87 ++++++++++++++++++-----------------
 2 files changed, 45 insertions(+), 43 deletions(-)


base-commit: 6b4b543fa18757297ac756a6fe6e64f506b7844a
-- 
2.56.0

[PATCH 1/2] Bluetooth: L2CAP: Only accept LE responses to outstanding requests

[Zhao Li]

The LE and ECRED response handlers match channels by identifier without
checking whether they sent the request. An incoming channel stores the
identifier chosen by the peer, so an unsolicited response can bypass
deferred setup, delete an established channel, or leave an unaccepted
socket and channel allocated after l2cap_chan_del().

Extend the BR/EDR lookup from commit a518b9a9ef6e ("Bluetooth: L2CAP:
Only accept connection responses to outstanding requests") to LE.
Require the matching connect-request flag and add a reconfigure-request
flag. Check reconfigure identifiers under the channel lock, since
BT_RCVMTU can replace a request without the connection lock. Match all
members of a rejected ECRED request, which share one identifier.

A reconfigure request can outlive close(). Complete it even in
BT_DISCONN, and call the close callback after deletion to release an
orphaned socket. Otherwise deletion cancels the timer and removes the
channel before the disconnect response or the channel timeout can close
it.

AI assistance was used.

Fixes: f1496dee9cbd ("Bluetooth: Add initial code for LE L2CAP Connect Request")
Fixes: 71fb419724fa ("Bluetooth: Fix handling of L2CAP Command Reject over LE")
Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: <stable@vger.kernel.org> # 5.7.x: a518b9a9ef6e: Bluetooth: L2CAP: Only accept connection responses to outstanding requests
Cc: <stable@vger.kernel.org> # 5.7.x
Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
---
Built in patch order at W=1 with GCC 12.2 (x86_64 module, x86_64
built-in and i386 module) and Clang 19.1.7 (x86_64 module). The x86_64
module builds with both compilers also linked vmlinux and modules.
Sparse checked l2cap_core.c with C=2 after each patch without
diagnostics. Smatch reported the same pre-existing l2cap_chan_connect()
diagnostic as the base; no cross-function database was used.

Coccinelle 1.3's use_after_iter rule found nothing in the changed
functions. All changed functions were processed; five unchanged parser
gaps also occur in the base. Hardware validation has not been run.
This diff does not apply to stable branches as is; adapted backports
compiled at W=1 on the seven supported lines and, with patch 2's
backport applied, linked as modules. Their behaviour was not tested.

QEMU/vhci testing with patch 1 alone kept all 445 established channels
alive. In 40-pair close-before runs, failed reconfigure responses and
Command Rejects each reached 44 l2cap_sock_destruct calls with the fix
and four base boot-noise calls; success reached 43 on both. A 1200-pair
base run retained 1984 kB of gross Slab memory at listener close while
the fixed arms released memory. Kmemleak reported nothing even on the
leaking base, so it was not used for the leak verdict. A KCSAN kernel
with both patches, built without KASAN, ran the same close-before
workloads without data-race reports.

The stable request covers 5.7 and newer with the declared BR/EDR lookup
prerequisite or an equivalent backport. Older LE-only trees need an
adapted patch without ECRED hunks. The diff exceeds the stable guideline
of 100 lines including context.

 include/net/bluetooth/l2cap.h |  1 +
 net/bluetooth/l2cap_core.c    | 82 +++++++++++++++++------------------
 2 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 54eddcdd341f..2a05bda8ab43 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -756,6 +756,7 @@ enum {
 	FLAG_DEFER_SETUP,
 	FLAG_LE_CONN_REQ_SENT,
 	FLAG_ECRED_CONN_REQ_SENT,
+	FLAG_ECRED_RECONF_REQ_SENT,
 	FLAG_PENDING_SECURITY,
 	FLAG_HOLD_HCI_CONN,
 	FLAG_DEL,
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4dd56e0074fd..a21b2d70ad8a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -144,24 +144,12 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
 	return c;
 }
 
-static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
-						    u8 ident)
-	__must_hold(&conn->lock)
-{
-	struct l2cap_chan *c;
-
-	list_for_each_entry(c, &conn->chan_l, list) {
-		if (c->ident == ident)
-			return c;
-	}
-	return NULL;
-}
-
 /* Our connection request has been sent and is still outstanding */
 static bool l2cap_chan_connect_pending(struct l2cap_chan *chan)
 {
 	return chan->state == BT_CONNECT &&
-	       test_bit(CONF_CONNECT_PEND, &chan->conf_state);
+	       (test_bit(CONF_CONNECT_PEND, &chan->conf_state) ||
+		test_bit(FLAG_LE_CONN_REQ_SENT, &chan->flags));
 }
 
 /* Other channels may hold the same ident for a response they still have
@@ -174,7 +162,7 @@ static struct l2cap_chan *__l2cap_get_connect_chan_by_ident(struct l2cap_conn *c
 	struct l2cap_chan *c;
 
 	list_for_each_entry(c, &conn->chan_l, list) {
-		if (c->ident == ident && l2cap_chan_connect_pending(c))
+		if (l2cap_chan_connect_pending(c) && c->ident == ident)
 			return c;
 	}
 	return NULL;
@@ -5073,7 +5061,7 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
 	BT_DBG("dcid 0x%4.4x mtu %u mps %u credits %u result 0x%2.2x",
 	       dcid, mtu, mps, credits, result);
 
-	chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
+	chan = __l2cap_get_connect_chan_by_ident(conn, cmd->ident);
 	if (!chan)
 		return -EBADSLT;
 
@@ -5621,9 +5609,10 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
 		struct l2cap_chan *orig;
 		u16 dcid;
 
-		if (chan->ident != cmd->ident ||
+		if (chan->state != BT_CONNECT ||
 		    chan->mode != L2CAP_MODE_EXT_FLOWCTL ||
-		    chan->state == BT_CONNECTED)
+		    !test_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags) ||
+		    chan->ident != cmd->ident)
 			continue;
 
 		l2cap_chan_hold(chan);
@@ -5842,16 +5831,9 @@ static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("result 0x%4.4x", result);
 
-	if (!result) {
-		list_for_each_entry(chan, &conn->chan_l, list) {
-			if (chan->ident == cmd->ident)
-				chan->ident = 0;
-		}
-		return 0;
-	}
-
 	list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
-		if (chan->ident != cmd->ident)
+		/* Reconfiguration updates ident with only chan->lock held. */
+		if (data_race(READ_ONCE(chan->ident)) != cmd->ident)
 			continue;
 
 		if (!l2cap_chan_hold_unless_zero(chan))
@@ -5860,7 +5842,15 @@ static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
 
 		lockdep_assert_held(&chan->conn->lock);
 
-		l2cap_chan_del(chan, ECONNRESET);
+		if (chan->ident == cmd->ident &&
+		    test_and_clear_bit(FLAG_ECRED_RECONF_REQ_SENT, &chan->flags)) {
+			if (result) {
+				l2cap_chan_del(chan, ECONNRESET);
+				chan->ops->close(chan);
+			} else {
+				chan->ident = 0;
+			}
+		}
 
 		l2cap_chan_unlock(chan);
 		l2cap_chan_put(chan);
@@ -5875,26 +5865,35 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
 	__must_hold(&conn->lock)
 {
 	struct l2cap_cmd_rej_unk *rej = (struct l2cap_cmd_rej_unk *) data;
-	struct l2cap_chan *chan;
+	struct l2cap_chan *chan, *tmp;
 
 	if (cmd_len < sizeof(*rej))
 		return -EPROTO;
 
-	chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
-	if (!chan)
-		goto done;
+	list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
+		/* Reconfiguration updates ident with only chan->lock held. */
+		if (data_race(READ_ONCE(chan->ident)) != cmd->ident)
+			continue;
 
-	chan = l2cap_chan_hold_unless_zero(chan);
-	if (!chan)
-		goto done;
+		if (!l2cap_chan_hold_unless_zero(chan))
+			continue;
 
-	l2cap_chan_lock(chan);
-	lockdep_assert_held(&chan->conn->lock);
-	l2cap_chan_del(chan, ECONNREFUSED);
-	l2cap_chan_unlock(chan);
-	l2cap_chan_put(chan);
+		l2cap_chan_lock(chan);
+		lockdep_assert_held(&chan->conn->lock);
+
+		if (chan->ident == cmd->ident &&
+		    ((chan->state == BT_CONNECT &&
+		      (test_bit(FLAG_LE_CONN_REQ_SENT, &chan->flags) ||
+		       test_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))) ||
+		     test_bit(FLAG_ECRED_RECONF_REQ_SENT, &chan->flags))) {
+			l2cap_chan_del(chan, ECONNREFUSED);
+			chan->ops->close(chan);
+		}
+
+		l2cap_chan_unlock(chan);
+		l2cap_chan_put(chan);
+	}
 
-done:
 	return 0;
 }
 
@@ -7757,6 +7756,7 @@ static void l2cap_ecred_reconfigure(struct l2cap_chan *chan)
 	pdu->scid[0] = cpu_to_le16(chan->scid);
 
 	chan->ident = l2cap_get_ident(conn);
+	set_bit(FLAG_ECRED_RECONF_REQ_SENT, &chan->flags);
 
 	l2cap_send_cmd(conn, chan->ident, L2CAP_ECRED_RECONF_REQ,
 		       struct_size(pdu, scid, 1), pdu);
-- 
2.56.0

[PATCH 2/2] Bluetooth: L2CAP: Clear request identifiers on incoming LE connections

[Zhao Li]

Incoming LE and ECRED channels retain the peer's request identifier
after an immediate response. The deferred LE response also leaves the
identifier set, unlike the deferred ECRED response.

A later ECRED request can reuse that identifier and include an unrelated
channel in its shared-response handling. That walk matches on the
identifier alone, so an LE channel is selected too. If the old channel is
closing, the checks can refuse the new connection. The check at request
receipt rejects it with L2CAP_CR_LE_INVALID_PARAMS; the check before a
deferred response returns L2CAP_CR_LE_AUTHORIZATION.

Store the identifier only when deferring the response, and clear it
after sending the deferred LE response. Keep deferred ECRED identifiers
until the shared response is sent, since a partially accepted group
still needs them.

AI assistance was used.

Fixes: 27e2d4c8d28b ("Bluetooth: Add basic LE L2CAP connect request receiving support")
Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
---
Built in patch order at W=1 with GCC 12.2 (x86_64 module, x86_64
built-in and i386 module) and Clang 19.1.7 (x86_64 module). The x86_64
module builds with both compilers also linked vmlinux and modules.
Sparse checked l2cap_core.c with C=2 after each patch without
diagnostics. Smatch reported the same pre-existing l2cap_chan_connect()
diagnostic as the base; no cross-function database was used.

Coccinelle 1.3's use_after_iter rule found nothing in the changed
functions. All changed functions were processed; five unchanged parser
gaps also occur in the base. Hardware validation has not been run.

The two-patch QEMU/vhci arm kept all 445 established channels alive.
In runs where new ECRED requests reused the identifier of a closing LE
channel, the base and patch 1 alone refused 18 of 18 at each site:
with L2CAP_CR_LE_INVALID_PARAMS at receipt, or with
L2CAP_CR_LE_AUTHORIZATION in the deferred response. With this patch,
all 18 completed with L2CAP_CR_LE_SUCCESS at both sites. Two further
requests per run were refused identically on every kernel and are not
counted. Partially accepted groups were not tested.

A KCSAN kernel with both patches, built without KASAN, ran the patch 1
close-before workloads without data-race reports.

 net/bluetooth/l2cap_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a21b2d70ad8a..0b6eb082f3f7 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3972,6 +3972,7 @@ void __l2cap_le_connect_rsp_defer(struct l2cap_chan *chan)
 
 	l2cap_send_cmd(conn, chan->ident, L2CAP_LE_CONN_RSP, sizeof(rsp),
 		       &rsp);
+	chan->ident = 0;
 }
 
 static void l2cap_ecred_list_defer(struct l2cap_chan *chan, void *data)
@@ -5311,10 +5312,10 @@ 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)) {
+		chan->ident = cmd->ident;
 		l2cap_state_change(chan, BT_CONNECT2);
 		/* The following result value is actually not defined
 		 * for LE CoC but we use it to let the function know
@@ -5550,10 +5551,10 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
 
 		__set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
 
-		chan->ident = cmd->ident;
 		chan->mode = L2CAP_MODE_EXT_FLOWCTL;
 
 		if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
+			chan->ident = cmd->ident;
 			l2cap_state_change(chan, BT_CONNECT2);
 			defer = true;
 			chan->ops->defer(chan);
-- 
2.56.0