2023-07-07 02:46:49

by Xigang(Ted) Feng

[permalink] [raw]
Subject: [PATCH] Bluetooth: Fix for Bluetooth SIG test L2CAP/COS/CED/BI-02-C

This test case is for verifying the L2CAP signalling PDUs that have invalid length are properly handled.
With this patch, the "L2CAP: Command Reject" packet is sent correctly to the malformed signal packet
contained in "L2CAP: Connection Request" packet.

BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3, Part A page 1041

'When a packet is received with a Code field that is unknown or disallowed on the
signalling channel it is received on, an L2CAP_COMMAND_REJECT_RSP
packet (defined in Section 4.1) is sent in response.'

Before this patch:

> ACL Data RX: Handle 1 flags 0x02 dlen 15
L2CAP: Connection Request (0x02) ident 3 len 4
PSM: 1 (0x0001)
Source CID: 64
malformed signal packet
00 00 00 ...
< ACL Data TX: Handle 1 flags 0x00 dlen 16
L2CAP: Connection Response (0x03) ident 3 len 8
Destination CID: 64
Source CID: 64
Result: Connection successful (0x0000)
Status: No further information available (0x0000)
< ACL Data TX: Handle 1 flags 0x00 dlen 23
L2CAP: Configure Request (0x04) ident 3 len 15
Destination CID: 64
Flags: 0x0000
Option: Retransmission and Flow Control (0x04) [mandatory]
Mode: Basic (0x00)
TX window size: 0
Max transmit: 0
Retransmission timeout: 0
Monitor timeout: 0
Maximum PDU size: 0
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 1
Count: 2
> ACL Data RX: Handle 1 flags 0x02 dlen 25
L2CAP: Configure Response (0x05) ident 3 len 17
Source CID: 64
Flags: 0x0000
Result: Success (0x0000)
Option: Retransmission and Flow Control (0x04) [mandatory]
Mode: Basic (0x00)
TX window size: 0
Max transmit: 0
Retransmission timeout: 0
Monitor timeout: 0
Maximum PDU size: 0
< ACL Data TX: Handle 1 flags 0x00 dlen 12
L2CAP: Disconnection Request (0x06) ident 4 len 4
Destination CID: 64
Source CID: 64
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 1
Count: 1
> ACL Data RX: Handle 1 flags 0x02 dlen 12
L2CAP: Disconnection Response (0x07) ident 4 len 4
Destination CID: 64
Source CID: 64
< HCI Command: Disconnect (0x01|0x0006) plen 3
Handle: 1
Reason: Remote User Terminated Connection (0x13)

After this patch:

> ACL Data RX: Handle 1 flags 0x02 dlen 15
L2CAP: Connection Request (0x02) ident 3 len 4
PSM: 4113 (0x1011)
Source CID: 64
malformed signal packet
00 00 00 ...
< ACL Data TX: Handle 1 flags 0x00 dlen 16
L2CAP: Connection Response (0x03) ident 3 len 8
Destination CID: 64
Source CID: 64
Result: Connection successful (0x0000)
Status: No further information available (0x0000)
< ACL Data TX: Handle 1 flags 0x00 dlen 23
L2CAP: Configure Request (0x04) ident 3 len 15
Destination CID: 64
Flags: 0x0000
Option: Retransmission and Flow Control (0x04) [mandatory]
Mode: Basic (0x00)
TX window size: 0
Max transmit: 0
Retransmission timeout: 0
Monitor timeout: 0
Maximum PDU size: 0
< ACL Data TX: Handle 1 flags 0x00 dlen 10
L2CAP: Command Reject (0x01) ident 0 len 2
Reason: Command not understood (0x0000)
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
Count: 2
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
Count: 1
> ACL Data RX: Handle 1 flags 0x02 dlen 25
L2CAP: Configure Response (0x05) ident 3 len 17
Source CID: 64
Flags: 0x0000
Result: Success (0x0000)
Option: Retransmission and Flow Control (0x04) [mandatory]
Mode: Basic (0x00)
TX window size: 0
Max transmit: 0
Retransmission timeout: 0
Monitor timeout: 0
Maximum PDU size: 0

Signed-off-by: Xigang(Ted) Feng <[email protected]>
---
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 17ca13e8c044..c3af7727ee1e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6534,6 +6534,14 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
skb_pull(skb, len);
}

+ if (skb->len) {
+ struct l2cap_cmd_rej_unk rej;
+
+ rej.reason = cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
+ l2cap_send_cmd(conn, 0, L2CAP_COMMAND_REJ,
+ sizeof(rej), &rej);
+ }
+
drop:
kfree_skb(skb);
}
--
2.34.1
________________________________
This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
________________________________


2023-07-07 03:01:45

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: Fix for Bluetooth SIG test L2CAP/COS/CED/BI-02-C

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: net/bluetooth/l2cap_core.c:6534
error: net/bluetooth/l2cap_core.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

2023-07-07 19:08:04

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix for Bluetooth SIG test L2CAP/COS/CED/BI-02-C

Hi Xigang,

On Thu, Jul 6, 2023 at 7:46 PM Xigang(Ted) Feng
<[email protected]> wrote:
>
> This test case is for verifying the L2CAP signalling PDUs that have invalid length are properly handled.
> With this patch, the "L2CAP: Command Reject" packet is sent correctly to the malformed signal packet
> contained in "L2CAP: Connection Request" packet.
>
> BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3, Part A page 1041
>
> 'When a packet is received with a Code field that is unknown or disallowed on the
> signalling channel it is received on, an L2CAP_COMMAND_REJECT_RSP
> packet (defined in Section 4.1) is sent in response.'
>
> Before this patch:
>
> > ACL Data RX: Handle 1 flags 0x02 dlen 15
> L2CAP: Connection Request (0x02) ident 3 len 4
> PSM: 1 (0x0001)
> Source CID: 64
> malformed signal packet
> 00 00 00 ...
> < ACL Data TX: Handle 1 flags 0x00 dlen 16
> L2CAP: Connection Response (0x03) ident 3 len 8
> Destination CID: 64
> Source CID: 64
> Result: Connection successful (0x0000)
> Status: No further information available (0x0000)
> < ACL Data TX: Handle 1 flags 0x00 dlen 23
> L2CAP: Configure Request (0x04) ident 3 len 15
> Destination CID: 64
> Flags: 0x0000
> Option: Retransmission and Flow Control (0x04) [mandatory]
> Mode: Basic (0x00)
> TX window size: 0
> Max transmit: 0
> Retransmission timeout: 0
> Monitor timeout: 0
> Maximum PDU size: 0
> > HCI Event: Number of Completed Packets (0x13) plen 5
> Num handles: 1
> Handle: 1
> Count: 2
> > ACL Data RX: Handle 1 flags 0x02 dlen 25
> L2CAP: Configure Response (0x05) ident 3 len 17
> Source CID: 64
> Flags: 0x0000
> Result: Success (0x0000)
> Option: Retransmission and Flow Control (0x04) [mandatory]
> Mode: Basic (0x00)
> TX window size: 0
> Max transmit: 0
> Retransmission timeout: 0
> Monitor timeout: 0
> Maximum PDU size: 0
> < ACL Data TX: Handle 1 flags 0x00 dlen 12
> L2CAP: Disconnection Request (0x06) ident 4 len 4
> Destination CID: 64
> Source CID: 64
> > HCI Event: Number of Completed Packets (0x13) plen 5
> Num handles: 1
> Handle: 1
> Count: 1
> > ACL Data RX: Handle 1 flags 0x02 dlen 12
> L2CAP: Disconnection Response (0x07) ident 4 len 4
> Destination CID: 64
> Source CID: 64
> < HCI Command: Disconnect (0x01|0x0006) plen 3
> Handle: 1
> Reason: Remote User Terminated Connection (0x13)
>
> After this patch:
>
> > ACL Data RX: Handle 1 flags 0x02 dlen 15
> L2CAP: Connection Request (0x02) ident 3 len 4
> PSM: 4113 (0x1011)
> Source CID: 64
> malformed signal packet
> 00 00 00 ...
> < ACL Data TX: Handle 1 flags 0x00 dlen 16
> L2CAP: Connection Response (0x03) ident 3 len 8
> Destination CID: 64
> Source CID: 64
> Result: Connection successful (0x0000)
> Status: No further information available (0x0000)
> < ACL Data TX: Handle 1 flags 0x00 dlen 23
> L2CAP: Configure Request (0x04) ident 3 len 15
> Destination CID: 64
> Flags: 0x0000
> Option: Retransmission and Flow Control (0x04) [mandatory]
> Mode: Basic (0x00)
> TX window size: 0
> Max transmit: 0
> Retransmission timeout: 0
> Monitor timeout: 0
> Maximum PDU size: 0

Looks like we are still sending a Configure Request which is sort of
useless if we are sending a Reject in the follow up command.

> < ACL Data TX: Handle 1 flags 0x00 dlen 10
> L2CAP: Command Reject (0x01) ident 0 len 2
> Reason: Command not understood (0x0000)
> > HCI Event: Number of Completed Packets (0x13) plen 5
> Num handles: 1
> Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> Count: 2
> > HCI Event: Number of Completed Packets (0x13) plen 5
> Num handles: 1
> Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> Count: 1
> > ACL Data RX: Handle 1 flags 0x02 dlen 25
> L2CAP: Configure Response (0x05) ident 3 len 17
> Source CID: 64
> Flags: 0x0000
> Result: Success (0x0000)
> Option: Retransmission and Flow Control (0x04) [mandatory]
> Mode: Basic (0x00)
> TX window size: 0
> Max transmit: 0
> Retransmission timeout: 0
> Monitor timeout: 0
> Maximum PDU size: 0
>
> Signed-off-by: Xigang(Ted) Feng <[email protected]>
> ---
> 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 17ca13e8c044..c3af7727ee1e 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -6534,6 +6534,14 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
> skb_pull(skb, len);
> }
>
> + if (skb->len) {
> + struct l2cap_cmd_rej_unk rej;
> +
> + rej.reason = cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
> + l2cap_send_cmd(conn, 0, L2CAP_COMMAND_REJ,
> + sizeof(rej), &rej);
> + }
> +
> drop:
> kfree_skb(skb);
> }
> --
> 2.34.1
> ________________________________
> This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
> ________________________________



--
Luiz Augusto von Dentz

2023-07-08 23:47:11

by Xigang(Ted) Feng

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: Fix for Bluetooth SIG test L2CAP/COS/CED/BI-02-C

Hi Luiz,

> On Thu, Jul 6, 2023 at 7:46 PM Xigang(Ted) Feng
> <[email protected]> wrote:
> >
> > This test case is for verifying the L2CAP signalling PDUs that have invalid length
> are properly handled.
> > With this patch, the "L2CAP: Command Reject" packet is sent correctly to the
> malformed signal packet
> > contained in "L2CAP: Connection Request" packet.
> >
> > BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3, Part A page 1041
> >
> > 'When a packet is received with a Code field that is unknown or disallowed on
> the
> > signalling channel it is received on, an L2CAP_COMMAND_REJECT_RSP
> > packet (defined in Section 4.1) is sent in response.'
> >
> > Before this patch:
> >
> > > ACL Data RX: Handle 1 flags 0x02 dlen 15
> > L2CAP: Connection Request (0x02) ident 3 len 4
> > PSM: 1 (0x0001)
> > Source CID: 64
> > malformed signal packet
> > 00 00 00 ...
> > < ACL Data TX: Handle 1 flags 0x00 dlen 16
> > L2CAP: Connection Response (0x03) ident 3 len 8
> > Destination CID: 64
> > Source CID: 64
> > Result: Connection successful (0x0000)
> > Status: No further information available (0x0000)
> > < ACL Data TX: Handle 1 flags 0x00 dlen 23
> > L2CAP: Configure Request (0x04) ident 3 len 15
> > Destination CID: 64
> > Flags: 0x0000
> > Option: Retransmission and Flow Control (0x04) [mandatory]
> > Mode: Basic (0x00)
> > TX window size: 0
> > Max transmit: 0
> > Retransmission timeout: 0
> > Monitor timeout: 0
> > Maximum PDU size: 0
> > > HCI Event: Number of Completed Packets (0x13) plen 5
> > Num handles: 1
> > Handle: 1
> > Count: 2
> > > ACL Data RX: Handle 1 flags 0x02 dlen 25
> > L2CAP: Configure Response (0x05) ident 3 len 17
> > Source CID: 64
> > Flags: 0x0000
> > Result: Success (0x0000)
> > Option: Retransmission and Flow Control (0x04) [mandatory]
> > Mode: Basic (0x00)
> > TX window size: 0
> > Max transmit: 0
> > Retransmission timeout: 0
> > Monitor timeout: 0
> > Maximum PDU size: 0
> > < ACL Data TX: Handle 1 flags 0x00 dlen 12
> > L2CAP: Disconnection Request (0x06) ident 4 len 4
> > Destination CID: 64
> > Source CID: 64
> > > HCI Event: Number of Completed Packets (0x13) plen 5
> > Num handles: 1
> > Handle: 1
> > Count: 1
> > > ACL Data RX: Handle 1 flags 0x02 dlen 12
> > L2CAP: Disconnection Response (0x07) ident 4 len 4
> > Destination CID: 64
> > Source CID: 64
> > < HCI Command: Disconnect (0x01|0x0006) plen 3
> > Handle: 1
> > Reason: Remote User Terminated Connection (0x13)
> >
> > After this patch:
> >
> > > ACL Data RX: Handle 1 flags 0x02 dlen 15
> > L2CAP: Connection Request (0x02) ident 3 len 4
> > PSM: 4113 (0x1011)
> > Source CID: 64
> > malformed signal packet
> > 00 00 00 ...
> > < ACL Data TX: Handle 1 flags 0x00 dlen 16
> > L2CAP: Connection Response (0x03) ident 3 len 8
> > Destination CID: 64
> > Source CID: 64
> > Result: Connection successful (0x0000)
> > Status: No further information available (0x0000)
> > < ACL Data TX: Handle 1 flags 0x00 dlen 23
> > L2CAP: Configure Request (0x04) ident 3 len 15
> > Destination CID: 64
> > Flags: 0x0000
> > Option: Retransmission and Flow Control (0x04) [mandatory]
> > Mode: Basic (0x00)
> > TX window size: 0
> > Max transmit: 0
> > Retransmission timeout: 0
> > Monitor timeout: 0
> > Maximum PDU size: 0
>
> Looks like we are still sending a Configure Request which is sort of
> useless if we are sending a Reject in the follow up command.

I'm following the test case "L2CAP/COS/CED/BI-02-C" in " L2CAP Test Suite" from Bluetooth SIG.
In Test Procedure, step 3 sends a L2CAP_CONNECTION_REQ with three extra bytes of 0.

Step 4 sends back L2CAP_CONNECTION_RSP to indicate the connection established successfully,
And send L2CAP_COMMAND_REJECT_RSP to reject the unknow command(the extra three bytes of 0) contained in L2CAP_CONNECTION_REQ.

In step 5, the Lower Tester can continue sending L2CAP_CONFIGURATION_REQ because the connection has been established successfully,
So in my opinion, the Configure Request sent by IUT is also a correct behaviour.

>
> > < ACL Data TX: Handle 1 flags 0x00 dlen 10
> > L2CAP: Command Reject (0x01) ident 0 len 2
> > Reason: Command not understood (0x0000)
> > > HCI Event: Number of Completed Packets (0x13) plen 5
> > Num handles: 1
> > Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> > Count: 2
> > > HCI Event: Number of Completed Packets (0x13) plen 5
> > Num handles: 1
> > Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> > Count: 1
> > > ACL Data RX: Handle 1 flags 0x02 dlen 25
> > L2CAP: Configure Response (0x05) ident 3 len 17
> > Source CID: 64
> > Flags: 0x0000
> > Result: Success (0x0000)
> > Option: Retransmission and Flow Control (0x04) [mandatory]
> > Mode: Basic (0x00)
> > TX window size: 0
> > Max transmit: 0
> > Retransmission timeout: 0
> > Monitor timeout: 0
> > Maximum PDU size: 0
> >
> > Signed-off-by: Xigang(Ted) Feng <[email protected]>
> > ---
> > 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 17ca13e8c044..c3af7727ee1e 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -6534,6 +6534,14 @@ static inline void l2cap_sig_channel(struct
> l2cap_conn *conn,
> > skb_pull(skb, len);
> > }
> >
> > + if (skb->len) {
> > + struct l2cap_cmd_rej_unk rej;
> > +
> > + rej.reason = cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
> > + l2cap_send_cmd(conn, 0, L2CAP_COMMAND_REJ,
> > + sizeof(rej), &rej);
> > + }
> > +
> > drop:
> > kfree_skb(skb);
> > }
> > --
> > 2.34.1
> --
> Luiz Augusto von Dentz

Best Regards,
Xigang
________________________________
This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
________________________________

2023-07-14 22:31:08

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix for Bluetooth SIG test L2CAP/COS/CED/BI-02-C

Hi,

On Sat, Jul 8, 2023 at 4:26 PM Xigang(Ted) Feng
<[email protected]> wrote:
>
> Hi Luiz,
>
> > On Thu, Jul 6, 2023 at 7:46 PM Xigang(Ted) Feng
> > <[email protected]> wrote:
> > >
> > > This test case is for verifying the L2CAP signalling PDUs that have invalid length
> > are properly handled.
> > > With this patch, the "L2CAP: Command Reject" packet is sent correctly to the
> > malformed signal packet
> > > contained in "L2CAP: Connection Request" packet.
> > >
> > > BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3, Part A page 1041
> > >
> > > 'When a packet is received with a Code field that is unknown or disallowed on
> > the
> > > signalling channel it is received on, an L2CAP_COMMAND_REJECT_RSP
> > > packet (defined in Section 4.1) is sent in response.'
> > >
> > > Before this patch:
> > >
> > > > ACL Data RX: Handle 1 flags 0x02 dlen 15
> > > L2CAP: Connection Request (0x02) ident 3 len 4
> > > PSM: 1 (0x0001)
> > > Source CID: 64
> > > malformed signal packet
> > > 00 00 00 ...
> > > < ACL Data TX: Handle 1 flags 0x00 dlen 16
> > > L2CAP: Connection Response (0x03) ident 3 len 8
> > > Destination CID: 64
> > > Source CID: 64
> > > Result: Connection successful (0x0000)
> > > Status: No further information available (0x0000)
> > > < ACL Data TX: Handle 1 flags 0x00 dlen 23
> > > L2CAP: Configure Request (0x04) ident 3 len 15
> > > Destination CID: 64
> > > Flags: 0x0000
> > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > Mode: Basic (0x00)
> > > TX window size: 0
> > > Max transmit: 0
> > > Retransmission timeout: 0
> > > Monitor timeout: 0
> > > Maximum PDU size: 0
> > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > Num handles: 1
> > > Handle: 1
> > > Count: 2
> > > > ACL Data RX: Handle 1 flags 0x02 dlen 25
> > > L2CAP: Configure Response (0x05) ident 3 len 17
> > > Source CID: 64
> > > Flags: 0x0000
> > > Result: Success (0x0000)
> > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > Mode: Basic (0x00)
> > > TX window size: 0
> > > Max transmit: 0
> > > Retransmission timeout: 0
> > > Monitor timeout: 0
> > > Maximum PDU size: 0
> > > < ACL Data TX: Handle 1 flags 0x00 dlen 12
> > > L2CAP: Disconnection Request (0x06) ident 4 len 4
> > > Destination CID: 64
> > > Source CID: 64
> > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > Num handles: 1
> > > Handle: 1
> > > Count: 1
> > > > ACL Data RX: Handle 1 flags 0x02 dlen 12
> > > L2CAP: Disconnection Response (0x07) ident 4 len 4
> > > Destination CID: 64
> > > Source CID: 64
> > > < HCI Command: Disconnect (0x01|0x0006) plen 3
> > > Handle: 1
> > > Reason: Remote User Terminated Connection (0x13)
> > >
> > > After this patch:
> > >
> > > > ACL Data RX: Handle 1 flags 0x02 dlen 15
> > > L2CAP: Connection Request (0x02) ident 3 len 4
> > > PSM: 4113 (0x1011)
> > > Source CID: 64
> > > malformed signal packet
> > > 00 00 00 ...
> > > < ACL Data TX: Handle 1 flags 0x00 dlen 16
> > > L2CAP: Connection Response (0x03) ident 3 len 8
> > > Destination CID: 64
> > > Source CID: 64
> > > Result: Connection successful (0x0000)
> > > Status: No further information available (0x0000)
> > > < ACL Data TX: Handle 1 flags 0x00 dlen 23
> > > L2CAP: Configure Request (0x04) ident 3 len 15
> > > Destination CID: 64
> > > Flags: 0x0000
> > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > Mode: Basic (0x00)
> > > TX window size: 0
> > > Max transmit: 0
> > > Retransmission timeout: 0
> > > Monitor timeout: 0
> > > Maximum PDU size: 0
> >
> > Looks like we are still sending a Configure Request which is sort of
> > useless if we are sending a Reject in the follow up command.
>
> I'm following the test case "L2CAP/COS/CED/BI-02-C" in " L2CAP Test Suite" from Bluetooth SIG.
> In Test Procedure, step 3 sends a L2CAP_CONNECTION_REQ with three extra bytes of 0.
>
> Step 4 sends back L2CAP_CONNECTION_RSP to indicate the connection established successfully,
> And send L2CAP_COMMAND_REJECT_RSP to reject the unknow command(the extra three bytes of 0) contained in L2CAP_CONNECTION_REQ.

Well I guess that is assuming the stack would consider the extra bytes
as a different command, in which case the PDU flow is correct but then
we probably need to check for unparsed bytes for all commands, not
just L2CAP_CONNECTION_REQ. Perhaps we need to take a look at the
errata which added this test to know exactly the intent of such test,
if I recall correctly L2CAP signalling channel supports more than one
outstanding command at the time, so perhaps it is valid to send
multiple commands in a row which is why this tests is done with extra
bytes at the end to simulate another command.

> In step 5, the Lower Tester can continue sending L2CAP_CONFIGURATION_REQ because the connection has been established successfully,
> So in my opinion, the Configure Request sent by IUT is also a correct behaviour.
>
> >
> > > < ACL Data TX: Handle 1 flags 0x00 dlen 10
> > > L2CAP: Command Reject (0x01) ident 0 len 2
> > > Reason: Command not understood (0x0000)
> > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > Num handles: 1
> > > Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> > > Count: 2
> > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > Num handles: 1
> > > Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> > > Count: 1
> > > > ACL Data RX: Handle 1 flags 0x02 dlen 25
> > > L2CAP: Configure Response (0x05) ident 3 len 17
> > > Source CID: 64
> > > Flags: 0x0000
> > > Result: Success (0x0000)
> > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > Mode: Basic (0x00)
> > > TX window size: 0
> > > Max transmit: 0
> > > Retransmission timeout: 0
> > > Monitor timeout: 0
> > > Maximum PDU size: 0
> > >
> > > Signed-off-by: Xigang(Ted) Feng <[email protected]>
> > > ---
> > > 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 17ca13e8c044..c3af7727ee1e 100644
> > > --- a/net/bluetooth/l2cap_core.c
> > > +++ b/net/bluetooth/l2cap_core.c
> > > @@ -6534,6 +6534,14 @@ static inline void l2cap_sig_channel(struct
> > l2cap_conn *conn,
> > > skb_pull(skb, len);
> > > }
> > >
> > > + if (skb->len) {
> > > + struct l2cap_cmd_rej_unk rej;
> > > +
> > > + rej.reason = cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
> > > + l2cap_send_cmd(conn, 0, L2CAP_COMMAND_REJ,
> > > + sizeof(rej), &rej);
> > > + }
> > > +
> > > drop:
> > > kfree_skb(skb);
> > > }
> > > --
> > > 2.34.1
> > --
> > Luiz Augusto von Dentz
>
> Best Regards,
> Xigang
> ________________________________
> This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
> ________________________________



--
Luiz Augusto von Dentz

2023-07-27 22:34:39

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix for Bluetooth SIG test L2CAP/COS/CED/BI-02-C

Hi Ted,

On Fri, Jul 14, 2023 at 3:27 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi,
>
> On Sat, Jul 8, 2023 at 4:26 PM Xigang(Ted) Feng
> <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > > On Thu, Jul 6, 2023 at 7:46 PM Xigang(Ted) Feng
> > > <[email protected]> wrote:
> > > >
> > > > This test case is for verifying the L2CAP signalling PDUs that have invalid length
> > > are properly handled.
> > > > With this patch, the "L2CAP: Command Reject" packet is sent correctly to the
> > > malformed signal packet
> > > > contained in "L2CAP: Connection Request" packet.
> > > >
> > > > BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3, Part A page 1041
> > > >
> > > > 'When a packet is received with a Code field that is unknown or disallowed on
> > > the
> > > > signalling channel it is received on, an L2CAP_COMMAND_REJECT_RSP
> > > > packet (defined in Section 4.1) is sent in response.'
> > > >
> > > > Before this patch:
> > > >
> > > > > ACL Data RX: Handle 1 flags 0x02 dlen 15
> > > > L2CAP: Connection Request (0x02) ident 3 len 4
> > > > PSM: 1 (0x0001)
> > > > Source CID: 64
> > > > malformed signal packet
> > > > 00 00 00 ...
> > > > < ACL Data TX: Handle 1 flags 0x00 dlen 16
> > > > L2CAP: Connection Response (0x03) ident 3 len 8
> > > > Destination CID: 64
> > > > Source CID: 64
> > > > Result: Connection successful (0x0000)
> > > > Status: No further information available (0x0000)
> > > > < ACL Data TX: Handle 1 flags 0x00 dlen 23
> > > > L2CAP: Configure Request (0x04) ident 3 len 15
> > > > Destination CID: 64
> > > > Flags: 0x0000
> > > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > > Mode: Basic (0x00)
> > > > TX window size: 0
> > > > Max transmit: 0
> > > > Retransmission timeout: 0
> > > > Monitor timeout: 0
> > > > Maximum PDU size: 0
> > > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > > Num handles: 1
> > > > Handle: 1
> > > > Count: 2
> > > > > ACL Data RX: Handle 1 flags 0x02 dlen 25
> > > > L2CAP: Configure Response (0x05) ident 3 len 17
> > > > Source CID: 64
> > > > Flags: 0x0000
> > > > Result: Success (0x0000)
> > > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > > Mode: Basic (0x00)
> > > > TX window size: 0
> > > > Max transmit: 0
> > > > Retransmission timeout: 0
> > > > Monitor timeout: 0
> > > > Maximum PDU size: 0
> > > > < ACL Data TX: Handle 1 flags 0x00 dlen 12
> > > > L2CAP: Disconnection Request (0x06) ident 4 len 4
> > > > Destination CID: 64
> > > > Source CID: 64
> > > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > > Num handles: 1
> > > > Handle: 1
> > > > Count: 1
> > > > > ACL Data RX: Handle 1 flags 0x02 dlen 12
> > > > L2CAP: Disconnection Response (0x07) ident 4 len 4
> > > > Destination CID: 64
> > > > Source CID: 64
> > > > < HCI Command: Disconnect (0x01|0x0006) plen 3
> > > > Handle: 1
> > > > Reason: Remote User Terminated Connection (0x13)
> > > >
> > > > After this patch:
> > > >
> > > > > ACL Data RX: Handle 1 flags 0x02 dlen 15
> > > > L2CAP: Connection Request (0x02) ident 3 len 4
> > > > PSM: 4113 (0x1011)
> > > > Source CID: 64
> > > > malformed signal packet
> > > > 00 00 00 ...
> > > > < ACL Data TX: Handle 1 flags 0x00 dlen 16
> > > > L2CAP: Connection Response (0x03) ident 3 len 8
> > > > Destination CID: 64
> > > > Source CID: 64
> > > > Result: Connection successful (0x0000)
> > > > Status: No further information available (0x0000)
> > > > < ACL Data TX: Handle 1 flags 0x00 dlen 23
> > > > L2CAP: Configure Request (0x04) ident 3 len 15
> > > > Destination CID: 64
> > > > Flags: 0x0000
> > > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > > Mode: Basic (0x00)
> > > > TX window size: 0
> > > > Max transmit: 0
> > > > Retransmission timeout: 0
> > > > Monitor timeout: 0
> > > > Maximum PDU size: 0
> > >
> > > Looks like we are still sending a Configure Request which is sort of
> > > useless if we are sending a Reject in the follow up command.
> >
> > I'm following the test case "L2CAP/COS/CED/BI-02-C" in " L2CAP Test Suite" from Bluetooth SIG.
> > In Test Procedure, step 3 sends a L2CAP_CONNECTION_REQ with three extra bytes of 0.
> >
> > Step 4 sends back L2CAP_CONNECTION_RSP to indicate the connection established successfully,
> > And send L2CAP_COMMAND_REJECT_RSP to reject the unknow command(the extra three bytes of 0) contained in L2CAP_CONNECTION_REQ.
>
> Well I guess that is assuming the stack would consider the extra bytes
> as a different command, in which case the PDU flow is correct but then
> we probably need to check for unparsed bytes for all commands, not
> just L2CAP_CONNECTION_REQ. Perhaps we need to take a look at the
> errata which added this test to know exactly the intent of such test,
> if I recall correctly L2CAP signalling channel supports more than one
> outstanding command at the time, so perhaps it is valid to send
> multiple commands in a row which is why this tests is done with extra
> bytes at the end to simulate another command.

Any chance to update these changes? Or are you not planning on
continuing working on them?

> > In step 5, the Lower Tester can continue sending L2CAP_CONFIGURATION_REQ because the connection has been established successfully,
> > So in my opinion, the Configure Request sent by IUT is also a correct behaviour.
> >
> > >
> > > > < ACL Data TX: Handle 1 flags 0x00 dlen 10
> > > > L2CAP: Command Reject (0x01) ident 0 len 2
> > > > Reason: Command not understood (0x0000)
> > > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > > Num handles: 1
> > > > Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> > > > Count: 2
> > > > > HCI Event: Number of Completed Packets (0x13) plen 5
> > > > Num handles: 1
> > > > Handle: 1 Address: 00:1B:DC:F4:B3:E1 (Vencer Co., Ltd.)
> > > > Count: 1
> > > > > ACL Data RX: Handle 1 flags 0x02 dlen 25
> > > > L2CAP: Configure Response (0x05) ident 3 len 17
> > > > Source CID: 64
> > > > Flags: 0x0000
> > > > Result: Success (0x0000)
> > > > Option: Retransmission and Flow Control (0x04) [mandatory]
> > > > Mode: Basic (0x00)
> > > > TX window size: 0
> > > > Max transmit: 0
> > > > Retransmission timeout: 0
> > > > Monitor timeout: 0
> > > > Maximum PDU size: 0
> > > >
> > > > Signed-off-by: Xigang(Ted) Feng <[email protected]>
> > > > ---
> > > > 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 17ca13e8c044..c3af7727ee1e 100644
> > > > --- a/net/bluetooth/l2cap_core.c
> > > > +++ b/net/bluetooth/l2cap_core.c
> > > > @@ -6534,6 +6534,14 @@ static inline void l2cap_sig_channel(struct
> > > l2cap_conn *conn,
> > > > skb_pull(skb, len);
> > > > }
> > > >
> > > > + if (skb->len) {
> > > > + struct l2cap_cmd_rej_unk rej;
> > > > +
> > > > + rej.reason = cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
> > > > + l2cap_send_cmd(conn, 0, L2CAP_COMMAND_REJ,
> > > > + sizeof(rej), &rej);
> > > > + }
> > > > +
> > > > drop:
> > > > kfree_skb(skb);
> > > > }
> > > > --
> > > > 2.34.1
> > > --
> > > Luiz Augusto von Dentz
> >
> > Best Regards,
> > Xigang
> > ________________________________
> > This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
> > ________________________________
>
>
>
> --
> Luiz Augusto von Dentz



--
Luiz Augusto von Dentz