2022-01-21 21:17:39

by D. Starke

[permalink] [raw]
Subject: [PATCH v2 1/1] tty: n_gsm: fix SW flow control encoding/handling

n_gsm is based on the 3GPP 07.010 and its newer version is the 3GPP 27.010.
See https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.aspx?specificationId=1516
The changes from 07.010 to 27.010 are non-functional. Therefore, I refer to
the newer 27.010 here. Chapter 5.2.7.3 states that DC1 (XON) and DC3 (XOFF)
are the control characters defined in ISO/IEC 646. These shall be quoted if
seen in the data stream to avoid interpretation as flow control characters.

ISO/IEC 646 refers to the set of ISO standards described as the ISO
7-bit coded character set for information interchange. Its final version
is also known as ITU T.50.
See https://www.itu.int/rec/T-REC-T.50-199209-I/en

To abide the standard it is needed to quote DC1 and DC3 correctly if these
are seen as data bytes and not as control characters. The current
implementation already tries to enforce this but fails to catch all
defined cases. 3GPP 27.010 chapter 5.2.7.3 clearly states that the most
significant bit shall be ignored for DC1 and DC3 handling. The current
implementation handles only the case with the most significant bit set 0.
Cases in which DC1 and DC3 have the most significant bit set 1 are left
unhandled.

This patch fixes this by masking the data bytes with ISO_IEC_646_MASK (only
the 7 least significant bits set 1) before comparing them with XON
(a.k.a. DC1) and XOFF (a.k.a. DC3) when testing which byte values need
quotation via byte stuffing.

Fixes: e1eaea46bb40 (tty: n_gsm line discipline, 2010-03-26)
Cc: [email protected]
Signed-off-by: Daniel Starke <[email protected]>
---
drivers/tty/n_gsm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index ba27b274c967..0b1808e3a912 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -322,6 +322,7 @@ static int addr_cnt;
#define GSM1_ESCAPE_BITS 0x20
#define XON 0x11
#define XOFF 0x13
+#define ISO_IEC_646_MASK 0x7F

static const struct tty_port_operations gsm_port_ops;

@@ -531,7 +532,8 @@ static int gsm_stuff_frame(const u8 *input, u8 *output, int len)
int olen = 0;
while (len--) {
if (*input == GSM1_SOF || *input == GSM1_ESCAPE
- || *input == XON || *input == XOFF) {
+ || (*input & ISO_IEC_646_MASK) == XON
+ || (*input & ISO_IEC_646_MASK) == XOFF) {
*output++ = GSM1_ESCAPE;
*output++ = *input++ ^ GSM1_ESCAPE_BITS;
olen++;
--
2.25.1


2022-01-26 21:24:33

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] tty: n_gsm: fix SW flow control encoding/handling

On Thu, Jan 20, 2022 at 02:18:57AM -0800, [email protected] wrote:
> n_gsm is based on the 3GPP 07.010 and its newer version is the 3GPP 27.010.
> See https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.aspx?specificationId=1516
> The changes from 07.010 to 27.010 are non-functional. Therefore, I refer to
> the newer 27.010 here. Chapter 5.2.7.3 states that DC1 (XON) and DC3 (XOFF)
> are the control characters defined in ISO/IEC 646. These shall be quoted if
> seen in the data stream to avoid interpretation as flow control characters.
>
> ISO/IEC 646 refers to the set of ISO standards described as the ISO
> 7-bit coded character set for information interchange. Its final version
> is also known as ITU T.50.
> See https://www.itu.int/rec/T-REC-T.50-199209-I/en
>
> To abide the standard it is needed to quote DC1 and DC3 correctly if these
> are seen as data bytes and not as control characters. The current
> implementation already tries to enforce this but fails to catch all
> defined cases. 3GPP 27.010 chapter 5.2.7.3 clearly states that the most
> significant bit shall be ignored for DC1 and DC3 handling. The current
> implementation handles only the case with the most significant bit set 0.
> Cases in which DC1 and DC3 have the most significant bit set 1 are left
> unhandled.
>
> This patch fixes this by masking the data bytes with ISO_IEC_646_MASK (only
> the 7 least significant bits set 1) before comparing them with XON
> (a.k.a. DC1) and XOFF (a.k.a. DC3) when testing which byte values need
> quotation via byte stuffing.
>
> Fixes: e1eaea46bb40 (tty: n_gsm line discipline, 2010-03-26)

Nit, no need for a date here, our tools get mad about stuff like this.
Look at the proper format for "Fixes:" line in the documentation.

thanks,

greg k-h