[PATCH v2] Bluetooth: L2CAP: fix memory leak from repeated configuration of a connected ERTM channel

[PATCH v2] Bluetooth: L2CAP: fix memory leak from repeated configuration of a connected ERTM channel

[Henry Martin]

l2cap_config_req() accepts a CONF_REQ on a channel that is already
BT_CONNECTED (the state check explicitly allows it), and
l2cap_config_rsp() has no state check at all. A misbehaving BR/EDR peer
can therefore drive a connected ERTM channel through the configuration
procedure any number of times:

 1. CONF_REQ on the connected channel re-arms CONF_OUTPUT_DONE
    (l2cap_parse_conf_req() sets it on a successful parse);
 2. CONF_RSP(SUCCESS) sets CONF_INPUT_DONE and, because
    CONF_OUTPUT_DONE is set, calls l2cap_ertm_init() once more.

Every re-entry of l2cap_ertm_init() leaks memory and resets the ERTM
state machine on a live channel:

  - chan->sdu = NULL drops a partially reassembled skb (leak);
  - skb_queue_head_init(&chan->tx_q / &chan->srej_q) detaches every
    queued frame from the lists (leak);
  - l2cap_seq_list_init() overwrites chan->srej_list.list /
    chan->retrans_list.list without freeing the previous arrays (leak
    of two kmalloc arrays per round).

Normally l2cap_chan_ready() clears conf_state so the re-entry cannot
happen; but l2cap_chan_ready() returns early when the channel is
already BT_CONNECTED, so a peer that keeps the channel connected
while replaying CONF_REQ/CONF_RSP pairs bypasses that reset entirely.

Drop BT_CONNECTED from the states that accept a CONF_REQ, and have
l2cap_config_rsp() bail out when the channel is connected, keeping the
configuration procedure strictly pre-connection.

This issue was discovered by Tencent CodeBuddy Security.

Fixes: 96298f640104 ("Bluetooth: L2CAP: handle l2cap config request during open state")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Rebase onto current bluetooth-next (the lookup helpers no longer
    take the channel lock, so the config_rsp bail now drops the
    reference with l2cap_chan_put() only, without
    l2cap_chan_unlock()).  Fixes the CI apply failure against
    bluetooth-next HEAD.

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

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 0000000000000..0000000000000 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4591,8 +4591,7 @@
 
 	lockdep_assert_held(&chan->conn->lock);
 
-	if (chan->state != BT_CONFIG && chan->state != BT_CONNECT2 &&
-	    chan->state != BT_CONNECTED) {
+	if (chan->state != BT_CONFIG && chan->state != BT_CONNECT2) {
 		cmd_reject_invalid_cid(conn, cmd->ident, chan->scid,
 				       chan->dcid);
 		goto unlock;
@@ -4709,6 +4708,11 @@
 	if (!chan)
 		return 0;
 
+	if (chan->state == BT_CONNECTED) {
+		l2cap_chan_put(chan);
+		return 0;
+	}
+
 	l2cap_chan_lock(chan);
 
 	lockdep_assert_held(&chan->conn->lock);
---
-- 
2.43.7

RE: [v2] Bluetooth: L2CAP: fix memory leak from repeated configuration of a connected ERTM channel

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1393 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/series/1195059/

---Test result---

Test Summary:
CheckPatch                    PASS      0.54 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.11 seconds
GitLint                       FAIL      0.26 seconds
SubjectPrefix                 PASS      0.10 seconds
BuildKernel                   PASS      18.57 seconds
CheckAllWarning               PASS      21.91 seconds
CheckSparse                   PASS      24.23 seconds
BuildKernel32                 PASS      19.93 seconds
CheckKernelLLVM               PASS      21.51 seconds
TestRunnerSetup               PASS      435.33 seconds
TestRunner_l2cap-tester       PASS      12.79 seconds
IncrementalBuild              PASS      18.12 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2] Bluetooth: L2CAP: fix memory leak from repeated configuration of a connected ERTM channel

1: T1 Title exceeds max length (94>80): "[v2] Bluetooth: L2CAP: fix memory leak from repeated configuration of a connected ERTM channel"


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

---
Regards,
Linux Bluetooth