Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: rfcomm: Accept any XON/XOFF char
@ 2025-03-31 13:15 Frédéric Danis
  2025-03-31 13:30 ` Luiz Augusto von Dentz
  2025-03-31 14:03 ` bluez.test.bot
  0 siblings, 2 replies; 4+ messages in thread
From: Frédéric Danis @ 2025-03-31 13:15 UTC (permalink / raw)
  To: linux-bluetooth

The latest version of PTS test RFCOMM/DEVA-DEVB/RFC/BV-17-C
(RFCOMM v11.7.6.3) used unusual chars for XON (0x28 instead of
0x11) and XOFF (0xC8 instead of 0x13) and expect a reply with RPN
parameters set for XON and XOFF.

Current btmon traces:
> ACL Data RX: Handle 11 flags 0x02 dlen 18
      Channel: 64 len 14 [PSM 3 mode Basic (0x00)] {chan 0}
      RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
         Address: 0x03 cr 1 dlci 0x00
         Control: 0xef poll/final 0
         Length: 10
         FCS: 0x70
         MCC Message type: Remote Port Negotiation Command CMD (0x24)
           Length: 8
           dlci 32
           br 3 db 3 sb 0 p 0 pt 0 xi 0 xo 0
           rtri 0 rtro 0 rtci 0 rtco 0 xon 40 xoff 200
           pm 0xff7f
< ACL Data TX: Handle 11 flags 0x00 dlen 18
      Channel: 64 len 14 [PSM 3 mode Basic (0x00)] {chan 0}
      RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
         Address: 0x01 cr 0 dlci 0x00
         Control: 0xef poll/final 0
         Length: 10
         FCS: 0xaa
         MCC Message type: Remote Port Negotiation Command RSP (0x24)
           Length: 8
           dlci 32
           br 3 db 3 sb 0 p 0 pt 0 xi 0 xo 0
           rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
           pm 0x3f1f

Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
---
 net/bluetooth/rfcomm/core.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index ad5177e3a69b..0c0525939aa0 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1562,23 +1562,15 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
 		}
 	}
 
-	if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XON)) {
+	if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XON))
 		xon_char = rpn->xon_char;
-		if (xon_char != RFCOMM_RPN_XON_CHAR) {
-			BT_DBG("RPN XON char mismatch 0x%x", xon_char);
-			xon_char = RFCOMM_RPN_XON_CHAR;
-			rpn_mask ^= RFCOMM_RPN_PM_XON;
-		}
-	}
+	else
+		rpn_mask ^= RFCOMM_RPN_PM_XON;
 
-	if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XOFF)) {
+	if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XOFF))
 		xoff_char = rpn->xoff_char;
-		if (xoff_char != RFCOMM_RPN_XOFF_CHAR) {
-			BT_DBG("RPN XOFF char mismatch 0x%x", xoff_char);
-			xoff_char = RFCOMM_RPN_XOFF_CHAR;
-			rpn_mask ^= RFCOMM_RPN_PM_XOFF;
-		}
-	}
+	else
+		rpn_mask ^= RFCOMM_RPN_PM_XOFF;
 
 rpn_out:
 	rfcomm_send_rpn(s, 0, dlci, bit_rate, data_bits, stop_bits,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: rfcomm: Accept any XON/XOFF char
  2025-03-31 13:15 [PATCH] Bluetooth: rfcomm: Accept any XON/XOFF char Frédéric Danis
@ 2025-03-31 13:30 ` Luiz Augusto von Dentz
  2025-04-01  7:19   ` Frédéric Danis
  2025-03-31 14:03 ` bluez.test.bot
  1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2025-03-31 13:30 UTC (permalink / raw)
  To: Frédéric Danis; +Cc: linux-bluetooth

Hi Frédéric,

On Mon, Mar 31, 2025 at 9:15 AM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> The latest version of PTS test RFCOMM/DEVA-DEVB/RFC/BV-17-C
> (RFCOMM v11.7.6.3) used unusual chars for XON (0x28 instead of
> 0x11) and XOFF (0xC8 instead of 0x13) and expect a reply with RPN
> parameters set for XON and XOFF.
>
> Current btmon traces:
> > ACL Data RX: Handle 11 flags 0x02 dlen 18
>       Channel: 64 len 14 [PSM 3 mode Basic (0x00)] {chan 0}
>       RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
>          Address: 0x03 cr 1 dlci 0x00
>          Control: 0xef poll/final 0
>          Length: 10
>          FCS: 0x70
>          MCC Message type: Remote Port Negotiation Command CMD (0x24)
>            Length: 8
>            dlci 32
>            br 3 db 3 sb 0 p 0 pt 0 xi 0 xo 0
>            rtri 0 rtro 0 rtci 0 rtco 0 xon 40 xoff 200
>            pm 0xff7f
> < ACL Data TX: Handle 11 flags 0x00 dlen 18
>       Channel: 64 len 14 [PSM 3 mode Basic (0x00)] {chan 0}
>       RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
>          Address: 0x01 cr 0 dlci 0x00
>          Control: 0xef poll/final 0
>          Length: 10
>          FCS: 0xaa
>          MCC Message type: Remote Port Negotiation Command RSP (0x24)
>            Length: 8
>            dlci 32
>            br 3 db 3 sb 0 p 0 pt 0 xi 0 xo 0
>            rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
>            pm 0x3f1f
>
> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
> ---
>  net/bluetooth/rfcomm/core.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index ad5177e3a69b..0c0525939aa0 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -1562,23 +1562,15 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
>                 }
>         }
>
> -       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XON)) {
> +       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XON))
>                 xon_char = rpn->xon_char;
> -               if (xon_char != RFCOMM_RPN_XON_CHAR) {
> -                       BT_DBG("RPN XON char mismatch 0x%x", xon_char);
> -                       xon_char = RFCOMM_RPN_XON_CHAR;
> -                       rpn_mask ^= RFCOMM_RPN_PM_XON;
> -               }
> -       }

So is the assumption that we don't need to check the actual character
sent part of the spec? If it is then it is probably worth quoting it,
otherwise I'd update the check to include both old and new chars.

> +       else
> +               rpn_mask ^= RFCOMM_RPN_PM_XON;
>
> -       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XOFF)) {
> +       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XOFF))
>                 xoff_char = rpn->xoff_char;
> -               if (xoff_char != RFCOMM_RPN_XOFF_CHAR) {
> -                       BT_DBG("RPN XOFF char mismatch 0x%x", xoff_char);
> -                       xoff_char = RFCOMM_RPN_XOFF_CHAR;
> -                       rpn_mask ^= RFCOMM_RPN_PM_XOFF;
> -               }
> -       }
> +       else
> +               rpn_mask ^= RFCOMM_RPN_PM_XOFF;
>
>  rpn_out:
>         rfcomm_send_rpn(s, 0, dlci, bit_rate, data_bits, stop_bits,
> --
> 2.43.0
>
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: Bluetooth: rfcomm: Accept any XON/XOFF char
  2025-03-31 13:15 [PATCH] Bluetooth: rfcomm: Accept any XON/XOFF char Frédéric Danis
  2025-03-31 13:30 ` Luiz Augusto von Dentz
@ 2025-03-31 14:03 ` bluez.test.bot
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2025-03-31 14:03 UTC (permalink / raw)
  To: linux-bluetooth, frederic.danis

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.38 seconds
GitLint                       PENDING   0.31 seconds
SubjectPrefix                 PASS      0.34 seconds
BuildKernel                   PASS      24.51 seconds
CheckAllWarning               PASS      33.11 seconds
CheckSparse                   PASS      30.23 seconds
BuildKernel32                 PASS      24.25 seconds
TestRunnerSetup               PASS      432.43 seconds
TestRunner_l2cap-tester       PASS      21.47 seconds
TestRunner_iso-tester         PASS      29.01 seconds
TestRunner_bnep-tester        PASS      4.71 seconds
TestRunner_mgmt-tester        FAIL      120.57 seconds
TestRunner_rfcomm-tester      PASS      7.85 seconds
TestRunner_sco-tester         PASS      12.58 seconds
TestRunner_ioctl-tester       PASS      8.27 seconds
TestRunner_mesh-tester        PASS      6.09 seconds
TestRunner_smp-tester         PASS      7.17 seconds
TestRunner_userchan-tester    PASS      4.93 seconds
IncrementalBuild              PENDING   0.65 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: 485 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
LL Privacy - Set Flags 2 (Enable RL)                 Failed       0.153 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: rfcomm: Accept any XON/XOFF char
  2025-03-31 13:30 ` Luiz Augusto von Dentz
@ 2025-04-01  7:19   ` Frédéric Danis
  0 siblings, 0 replies; 4+ messages in thread
From: Frédéric Danis @ 2025-04-01  7:19 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On 31/03/2025 15:30, Luiz Augusto von Dentz wrote:
> Hi Frédéric,
>
> On Mon, Mar 31, 2025 at 9:15 AM Frédéric Danis
> <frederic.danis@collabora.com> wrote:
>> The latest version of PTS test RFCOMM/DEVA-DEVB/RFC/BV-17-C
>> (RFCOMM v11.7.6.3) used unusual chars for XON (0x28 instead of
>> 0x11) and XOFF (0xC8 instead of 0x13) and expect a reply with RPN
>> parameters set for XON and XOFF.
>>
>> Current btmon traces:
>>> ACL Data RX: Handle 11 flags 0x02 dlen 18
>>        Channel: 64 len 14 [PSM 3 mode Basic (0x00)] {chan 0}
>>        RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
>>           Address: 0x03 cr 1 dlci 0x00
>>           Control: 0xef poll/final 0
>>           Length: 10
>>           FCS: 0x70
>>           MCC Message type: Remote Port Negotiation Command CMD (0x24)
>>             Length: 8
>>             dlci 32
>>             br 3 db 3 sb 0 p 0 pt 0 xi 0 xo 0
>>             rtri 0 rtro 0 rtci 0 rtco 0 xon 40 xoff 200
>>             pm 0xff7f
>> < ACL Data TX: Handle 11 flags 0x00 dlen 18
>>        Channel: 64 len 14 [PSM 3 mode Basic (0x00)] {chan 0}
>>        RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
>>           Address: 0x01 cr 0 dlci 0x00
>>           Control: 0xef poll/final 0
>>           Length: 10
>>           FCS: 0xaa
>>           MCC Message type: Remote Port Negotiation Command RSP (0x24)
>>             Length: 8
>>             dlci 32
>>             br 3 db 3 sb 0 p 0 pt 0 xi 0 xo 0
>>             rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
>>             pm 0x3f1f
>>
>> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
>> ---
>>   net/bluetooth/rfcomm/core.c | 20 ++++++--------------
>>   1 file changed, 6 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index ad5177e3a69b..0c0525939aa0 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -1562,23 +1562,15 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
>>                  }
>>          }
>>
>> -       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XON)) {
>> +       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XON))
>>                  xon_char = rpn->xon_char;
>> -               if (xon_char != RFCOMM_RPN_XON_CHAR) {
>> -                       BT_DBG("RPN XON char mismatch 0x%x", xon_char);
>> -                       xon_char = RFCOMM_RPN_XON_CHAR;
>> -                       rpn_mask ^= RFCOMM_RPN_PM_XON;
>> -               }
>> -       }
> So is the assumption that we don't need to check the actual character
> sent part of the spec? If it is then it is probably worth quoting it,
> otherwise I'd update the check to include both old and new chars.

Afaiu the RFCOMM and GSM 07.10 specs I would say that nothing defines 
that the XON/XOFF characters are limited to some values, but only 
defines default values for XON and XOFF (see 5.4.6.3.9 Remote Port 
Negotiation Command (RPN) in 
https://www.etsi.org/deliver/etsi_ts/101300_101399/101369/06.03.00_60/ts_101369v060300p.pdf).
I haven't found a clear "quotable" definition of this in the GSM 07.10.

On the other hand adding the characters used by PTS to the checks will 
not prevent future changes.

Any advice on this?

>> +       else
>> +               rpn_mask ^= RFCOMM_RPN_PM_XON;
>>
>> -       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XOFF)) {
>> +       if (rpn->param_mask & cpu_to_le16(RFCOMM_RPN_PM_XOFF))
>>                  xoff_char = rpn->xoff_char;
>> -               if (xoff_char != RFCOMM_RPN_XOFF_CHAR) {
>> -                       BT_DBG("RPN XOFF char mismatch 0x%x", xoff_char);
>> -                       xoff_char = RFCOMM_RPN_XOFF_CHAR;
>> -                       rpn_mask ^= RFCOMM_RPN_PM_XOFF;
>> -               }
>> -       }
>> +       else
>> +               rpn_mask ^= RFCOMM_RPN_PM_XOFF;
>>
>>   rpn_out:
>>          rfcomm_send_rpn(s, 0, dlci, bit_rate, data_bits, stop_bits,
>> --
>> 2.43.0
>>
>>
>

-- 
Frédéric Danis
Senior Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom
Registered in England & Wales, no. 5513718


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-04-01  7:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 13:15 [PATCH] Bluetooth: rfcomm: Accept any XON/XOFF char Frédéric Danis
2025-03-31 13:30 ` Luiz Augusto von Dentz
2025-04-01  7:19   ` Frédéric Danis
2025-03-31 14:03 ` bluez.test.bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox