[v3] Bluetooth: L2CAP: Fix L2CAP_ECRED_CONN_RSP response

Commit Message

Luiz Augusto von Dentz Feb. 18, 2025, 10:28 p.m. UTC

From: Luiz Augusto von Dentz <[e-mail]>

L2CAP_ECRED_CONN_RSP needs to respond DCID in the same order received as
SCID but the order is reversed due to use of list_add which actually
prepend channels to the list so the response is reversed:

> ACL Data RX: Handle 16 flags 0x02 dlen 26
      LE L2CAP: Enhanced Credit Connection Request (0x17) ident 2 len 18
        PSM: 39 (0x0027)
        MTU: 256
        MPS: 251
        Credits: 65535
        Source CID: 116
        Source CID: 117
        Source CID: 118
        Source CID: 119
        Source CID: 120
< ACL Data TX: Handle 16 flags 0x00 dlen 26
      LE L2CAP: Enhanced Credit Connection Response (0x18) ident 2 len 18
        MTU: 517
        MPS: 247
        Credits: 3
        Result: Connection successful (0x0000)
        Destination CID: 68
        Destination CID: 67
        Destination CID: 66
        Destination CID: 65
        Destination CID: 64

Also make sure the response don't include channels that are not on
BT_CONNECT2 since the chan->ident can be set to the same value as in the
following trace:

< ACL Data TX: Handle 16 flags 0x00 dlen 12
      LE L2CAP: LE Flow Control Credit (0x16) ident 6 len 4
        Source CID: 64
        Credits: 1
...
> ACL Data RX: Handle 16 flags 0x02 dlen 18
      LE L2CAP: Enhanced Credit Connection Request (0x17) ident 6 len 10
        PSM: 39 (0x0027)
        MTU: 517
        MPS: 251
        Credits: 255
        Source CID: 70
< ACL Data TX: Handle 16 flags 0x00 dlen 20
      LE L2CAP: Enhanced Credit Connection Response (0x18) ident 6 len 12
        MTU: 517
        MPS: 247
        Credits: 3
        Result: Connection successful (0x0000)
        Destination CID: 64
        Destination CID: 68

Closes: https://github.com/bluez/bluez/issues/1094
Fixes: 9aa9d9473f15 ("Bluetooth: L2CAP: Fix responding with wrong PDU type")
Signed-off-by: Luiz Augusto von Dentz <[e-mail]>
---
 net/bluetooth/l2cap_core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

[e-mail] Feb. 18, 2025, 10:56 p.m. UTC | #1

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=935310

---Test result---

Test Summary:
CheckPatch                    PENDING   0.37 seconds
GitLint                       PENDING   0.27 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      25.41 seconds
CheckAllWarning               PASS      27.87 seconds
CheckSparse                   PASS      31.08 seconds
BuildKernel32                 PASS      24.72 seconds
TestRunnerSetup               PASS      436.39 seconds
TestRunner_l2cap-tester       PASS      20.98 seconds
TestRunner_iso-tester         PASS      36.39 seconds
TestRunner_bnep-tester        PASS      4.81 seconds
TestRunner_mgmt-tester        FAIL      120.31 seconds
TestRunner_rfcomm-tester      PASS      11.69 seconds
TestRunner_sco-tester         PASS      9.70 seconds
TestRunner_ioctl-tester       PASS      8.55 seconds
TestRunner_mesh-tester        PASS      6.18 seconds
TestRunner_smp-tester         PASS      7.45 seconds
TestRunner_userchan-tester    PASS      5.13 seconds
IncrementalBuild              PENDING   0.99 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 482 (98.4%), Failed: 4, Not Run: 4

Failed Test Cases
LL Privacy - Add Device 3 (AL is full)               Failed       0.199 seconds
LL Privacy - Set Flags 3 (2 Devices to RL)           Failed       0.186 seconds
LL Privacy - Start Discovery 1 (Disable RL)          Failed       0.166 seconds
LL Privacy - Start Discovery 2 (Disable RL)          Failed       0.194 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

[e-mail] Feb. 20, 2025, 6:10 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Tue, 18 Feb 2025 17:28:01 -0500 you wrote:
> From: Luiz Augusto von Dentz <[e-mail]>
> 
> L2CAP_ECRED_CONN_RSP needs to respond DCID in the same order received as
> SCID but the order is reversed due to use of list_add which actually
> prepend channels to the list so the response is reversed:
> 
> > ACL Data RX: Handle 16 flags 0x02 dlen 26
>       LE L2CAP: Enhanced Credit Connection Request (0x17) ident 2 len 18
>         PSM: 39 (0x0027)
>         MTU: 256
>         MPS: 251
>         Credits: 65535
>         Source CID: 116
>         Source CID: 117
>         Source CID: 118
>         Source CID: 119
>         Source CID: 120
> < ACL Data TX: Handle 16 flags 0x00 dlen 26
>       LE L2CAP: Enhanced Credit Connection Response (0x18) ident 2 len 18
>         MTU: 517
>         MPS: 247
>         Credits: 3
>         Result: Connection successful (0x0000)
>         Destination CID: 68
>         Destination CID: 67
>         Destination CID: 66
>         Destination CID: 65
>         Destination CID: 64
> 
> [...]

Here is the summary with links:
  - [v3] Bluetooth: L2CAP: Fix L2CAP_ECRED_CONN_RSP response
    https://git.kernel.org/bluetooth/bluetooth-next/c/aa6629540b87

You are awesome, thank you!

Patch

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fec11e576f31..b22078b67972 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -632,7 +632,8 @@  void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 	    test_bit(FLAG_HOLD_HCI_CONN, &chan->flags))
 		hci_conn_hold(conn->hcon);
 
-	list_add(&chan->list, &conn->chan_l);
+	/* Append to the list since the order matters for ECRED */
+	list_add_tail(&chan->list, &conn->chan_l);
 }
 
 void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
@@ -3771,7 +3772,11 @@  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 (test_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
+	/* Check if channel for outgoing connection or if it wasn't deferred
+	 * since in those cases it must be skipped.
+	 */
+	if (test_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags) ||
+	    !test_and_clear_bit(FLAG_DEFER_SETUP, &chan->flags))
 		return;
 
 	/* Reset ident so only one response is sent */