From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 99BA921501 for ; Thu, 11 Apr 2024 19:55:38 +0200 (CEST) Received: (qmail 3855 invoked by uid 550); 11 Apr 2024 17:55:33 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 3809 invoked from network); 11 Apr 2024 17:55:32 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712858123; x=1713462923; darn=lists.openwall.com; h=to:subject:message-id:date:from:reply-to:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=cZ3YntELn9rbS5UPbF2eI9m+cFr7pJXPCkF9krU+YXs=; b=j4wflZdzf5Rn9/SXlQ9tmERWYc+jw1R3DGdxYvYS6UcWf+67NUICU+QLAXU61qQDDz DinZ7s9KCjeTj6pgCiQuwKUGZASDlY2znZH8ClwmjVRcNdpd7jKzq7ZaIU89acCdLV1k /t941Zxziiw4Cl1PygYrr5n/aN5YCLJFPgeQA15NdpM+bKGCrIlkK7rFfIVgvJcCfY3f Sx+P6stYdCEcbDWz9+g4eOqMEdjkiX1/bm1JDxO7sgip93NqJiIh8UjBuJGLT+6W7r/r DqxZAOF7/QoTw+JojOZ7Iws20OkmljrjRvEEaFvkrNmUyJLHa3LJFtbZ1GLt6l9dHUqD +RdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712858123; x=1713462923; h=to:subject:message-id:date:from:reply-to:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cZ3YntELn9rbS5UPbF2eI9m+cFr7pJXPCkF9krU+YXs=; b=R6Qj68hSwgYrOyJ8rc9J9sgKcc70h6rYz9mQzJm54AMps3VwpjQDXdkEq0x3x5I6/p cYrqXvZbc8Qz/Sf+UlV2H6ep4r1LhXg0o2/j8Yt6eBUtP0/jrzy3/oFYGXtW6XCZimYj ttAlwRWIEq/AVFq2A3K8Pf5aGAd3Iebg8hatAx1HGFhrQ/tyG0yNBiSoptKj65dMNGNx s4FrXHY0U5Si1JQrEXAsab9+fUZCiTWomMJGXGGcE1ab2+a4BqhzH4E7Dl33r3VuweCb iQw8LF/x1HGJWxIPZAtf9spQzUKnsK79PFq2uVYlpcQPOX3qSJKEj0p6omtcBg9/dtbD gWjQ== X-Gm-Message-State: AOJu0YzwEkm6bB2FZ5RaSEJxSLjl1NMPiK2RZ3EEMOZkxXXfHEfugQAJ dBgIe3fS3DnUBNubbegY6B6u+Lgh5if9q3Hkucnx/8hpsG8dQLqaC7GL4Roc+O8ORHqn/MncMkT wLsw9SyCAUTbmDt+u5NMQI4CKiElc7BP/ X-Google-Smtp-Source: AGHT+IF4R3hdiSeosZ/dTKR8Vl+iyJGekWwRspSjktQR7MKRi8hujURc331sLDyq68uCylfyR+gfULDbWIxhKNjFqe8= X-Received: by 2002:a05:6871:20a:b0:22e:9792:97e5 with SMTP id t10-20020a056871020a00b0022e979297e5mr258006oad.30.1712858123502; Thu, 11 Apr 2024 10:55:23 -0700 (PDT) MIME-Version: 1.0 References: <20240411142455.GA4163@brightrain.aerifal.cx> <20240411165556.GB4163@brightrain.aerifal.cx> <20240411172956.GC4163@brightrain.aerifal.cx> In-Reply-To: <20240411172956.GC4163@brightrain.aerifal.cx> From: Jeffrey Walton Date: Thu, 11 Apr 2024 13:55:12 -0400 Message-ID: To: musl@lists.openwall.com Content-Type: multipart/alternative; boundary="0000000000000095f40615d5dcec" Subject: Re: [musl] making termios BOTHER speeds work --0000000000000095f40615d5dcec Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Apr 11, 2024 at 1:30=E2=80=AFPM Rich Felker wrote= : > On Thu, Apr 11, 2024 at 12:55:56PM -0400, Rich Felker wrote: > > On Thu, Apr 11, 2024 at 10:24:56AM -0400, Rich Felker wrote: > > > Since it's come up again, I'm looking at what it would take to get > > > support for custom baud rates in termios working. This topic is > > > something of a mess, as it involves discrepancies between our termios > > > structure and the kernel termios/termios2 structures. > > > > > > Szabolcs Nagy did some of the initial research on the mismatched > > > definitions in 2016: https://www.openwall.com/lists/musl/2016/04/26/3 > > > > > > Basically, it looks like what happened was that we tried to match the > > > glibc generic ABI (an early goal of lots of stuff in musl) as long as > > > it lined up with the kernel termios (not termios2) ioctl structure, > > > but deviated when it wouldn't (powerpc had c_line and c_cc order > > > flipped and we followed kernel on that), and didn't do glibc > > > arch-specific mistakes (like mips omitting the __c_[io]speed fields). > > > > > > If we had used the kernel value of NCCS everywhere, rather than the > > > inflated glibc value of 32, we could add BOTHER support just by > > > attempting TCSETS2 using the caller's termios structure, and only > > > falling back if it doesn't work. In theory we *could* change to do > > > this now. The __c_[io]speed members are not in the public namespace, > > > and NCCS could be reduced to accommodate them as long as the overall > > > struct size was preserved. This might be ~ugly~ for programs built > > > with the old NCCS of 32, which might copy c_cc past its new end, but > > > they'd just be moving stuff to/from the reserved speed fields they > > > couldn't yet be using. The worst part about this seems to be that we'= d > > > be introducing more arch-specific versions of bits/termios.h, since > > > the "generic" definition we have now actually has different layout > > > depending on the arch's alignment requirements. I think this only > > > affects m68k (where it's 2 rather than 4 byte alignment for int), so > > > maybe it's not a big deal to add just one. > > > > > > The alternative is to only use the caller-provided termios in-place i= n > > > the case where we can get by without termios2 interfaces: that is, > > > when either BOTHER is not set (classic POSIX baud flags), or TCSETS2 > > > is not defined (plain termios already supports BOTHER for this arch). > > > Otherwise, translate to a kernel termios2 form, which really requires > > > nothing other than knowing an arch-defined offset for the speed > > > fields. > > > > > > For going the other direction (tcgetattr) it's even easier: we're > > > allowed to clobber the caller buffer, so just try TCGETS2 and move th= e > > > speeds from their kernel offset into the libc member offsset. > > > > > > I think this second approach is probably better, but I'm open to > > > reasons why it might not be. > > > > One thing I hadn't even considered yet is how the application is > > expected to set custom speeds. We don't expose BOTHER, and while we > > could expose it and put the c_[io]speed members in the public > > namespace for direct access, it's not clear that this is the right way > > to do it. > > > > glibc's approach seems to be having cfset[io]speed accept values other > > than the symbolic B constants, which POSIX allows and mentions in the > > RATIONALE: > > > > There is nothing to prevent an implementation accepting as an > > extension a number (such as 126), and since the encoding of the > > Bxxx symbols is not specified, this can be done to avoid > > introducing ambiguity. > > > > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/cfgetispeed.ht= ml > > > > This seems like it's the better approach. It does have values 0-15 and > > 4096-4111 as impossible-to-set because they overlap with B constants, > > but these are not useful speeds. > > OK, no, it doesn't. Only the nonstandard cfsetspeed on glibc accepts > actual numbers, and applies them to both input and output. The > standard cfset[io]speed functions only accept the symbolic B > constants. And... they seem to be storing symbolic B constants in the > c_[io]speed members, which seems wrong. >_< > > > Of course it might be useful to look at what applications expect to be > > able to do. > > Thus, applications using the glibc API here need BOTHER to be defined > and need to directly access c_[io]speed members. > > This seems like an ugly leak of implementation details, but I'm not > sure whether it would be useful to have API-incompatible support for > custom bauds. > I have never encountered a need for a custom baud rate due to standardized UART chips. There are probably some edge cases out there. I'd like to hear about them. Reading a baud rate from a config file that can be modified by a user introduces tainted inputs. I clamp the speed to a B-constant to cleanse mistakes and malicious inputs: unsigned int term_clamp_speed(unsigned int speed) { #if defined(B4000000) if (speed >=3D 4000000) return 4000000; else #endif #if defined(B3500000) if (speed >=3D 3500000) return 3500000; else #endif ... if (speed >=3D 57600) return 57600; else if (speed >=3D 38400) return 38400; else if (speed >=3D 19200) return 19200; else if (speed >=3D 9600) return 9600; else if (speed >=3D 4800) return 4800; else if (speed >=3D 2400) return 2400; else return 1200; } > Maybe we should check what the BSDs or any other implementations do > here... > Jeff --0000000000000095f40615d5dcec Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Apr 11, 2024 at 1:30=E2=80=AF= PM Rich Felker <dalias@libc.org&g= t; wrote:
On Thu= , Apr 11, 2024 at 12:55:56PM -0400, Rich Felker wrote:
> On Thu, Apr 11, 2024 at 10:24:56AM -0400, Rich Felker wrote:
> > Since it's come up again, I'm looking at what it would ta= ke to get
> > support for custom baud rates in termios working. This topic is > > something of a mess, as it involves discrepancies between our ter= mios
> > structure and the kernel termios/termios2 structures.
> >
> > Szabolcs Nagy did some of the initial research on the mismatched<= br> > > definitions in 2016: https://www.openwall.= com/lists/musl/2016/04/26/3
> >
> > Basically, it looks like what happened was that we tried to match= the
> > glibc generic ABI (an early goal of lots of stuff in musl) as lon= g as
> > it lined up with the kernel termios (not termios2) ioctl structur= e,
> > but deviated when it wouldn't (powerpc had c_line and c_cc or= der
> > flipped and we followed kernel on that), and didn't do glibc<= br> > > arch-specific mistakes (like mips omitting the __c_[io]speed fiel= ds).
> >
> > If we had used the kernel value of NCCS everywhere, rather than t= he
> > inflated glibc value of 32, we could add BOTHER support just by > > attempting TCSETS2 using the caller's termios structure, and = only
> > falling back if it doesn't work. In theory we *could* change = to do
> > this now. The __c_[io]speed members are not in the public namespa= ce,
> > and NCCS could be reduced to accommodate them as long as the over= all
> > struct size was preserved. This might be ~ugly~ for programs buil= t
> > with the old NCCS of 32, which might copy c_cc past its new end, = but
> > they'd just be moving stuff to/from the reserved speed fields= they
> > couldn't yet be using. The worst part about this seems to be = that we'd
> > be introducing more arch-specific versions of bits/termios.h, sin= ce
> > the "generic" definition we have now actually has diffe= rent layout
> > depending on the arch's alignment requirements. I think this = only
> > affects m68k (where it's 2 rather than 4 byte alignment for i= nt), so
> > maybe it's not a big deal to add just one.
> >
> > The alternative is to only use the caller-provided termios in-pla= ce in
> > the case where we can get by without termios2 interfaces: that is= ,
> > when either BOTHER is not set (classic POSIX baud flags), or TCSE= TS2
> > is not defined (plain termios already supports BOTHER for this ar= ch).
> > Otherwise, translate to a kernel termios2 form, which really requ= ires
> > nothing other than knowing an arch-defined offset for the speed > > fields.
> >
> > For going the other direction (tcgetattr) it's even easier: w= e're
> > allowed to clobber the caller buffer, so just try TCGETS2 and mov= e the
> > speeds from their kernel offset into the libc member offsset.
> >
> > I think this second approach is probably better, but I'm open= to
> > reasons why it might not be.
>
> One thing I hadn't even considered yet is how the application is > expected to set custom speeds. We don't expose BOTHER, and while w= e
> could expose it and put the c_[io]speed members in the public
> namespace for direct access, it's not clear that this is the right= way
> to do it.
>
> glibc's approach seems to be having cfset[io]speed accept values o= ther
> than the symbolic B constants, which POSIX allows and mentions in the<= br> > RATIONALE:
>
>=C2=A0 =C2=A0 =C2=A0There is nothing to prevent an implementation accep= ting as an
>=C2=A0 =C2=A0 =C2=A0extension a number (such as 126), and since the enc= oding of the
>=C2=A0 =C2=A0 =C2=A0Bxxx symbols is not specified, this can be done to = avoid
>=C2=A0 =C2=A0 =C2=A0introducing ambiguity.
>
> https://pubs.opengro= up.org/onlinepubs/9699919799/functions/cfgetispeed.html
>
> This seems like it's the better approach. It does have values 0-15= and
> 4096-4111 as impossible-to-set because they overlap with B constants,<= br> > but these are not useful speeds.

OK, no, it doesn't. Only the nonstandard cfsetspeed on glibc accepts actual numbers, and applies them to both input and output. The
standard cfset[io]speed functions only accept the symbolic B
constants. And... they seem to be storing symbolic B constants in the
c_[io]speed members, which seems wrong. >_<

> Of course it might be useful to look at what applications expect to be=
> able to do.

Thus, applications using the glibc API here need BOTHER to be defined
and need to directly access c_[io]speed members.

This seems like an ugly leak of implementation details, but I'm not
sure whether it would be useful to have API-incompatible support for
custom bauds.

I have never encountered = a need for a custom baud rate due to standardized UART chips. There are pro= bably some edge cases out there. I'd like to hear about them.
=

Reading a baud rate from a config file that can be modi= fied by a user introduces tainted inputs. I clamp the speed to a B-constant= to cleanse mistakes and malicious inputs:

unsigne= d int term_clamp_speed(unsigned int speed)
{
#if defined(B4000000)=C2=A0 =C2=A0 if (speed >=3D 4000000)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 re= turn 4000000;
=C2=A0 =C2=A0 else
#endif
#if defined(B3500000)
= =C2=A0 =C2=A0 if (speed >=3D 3500000)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ret= urn 3500000;
=C2=A0 =C2=A0 else
#endif
...
=C2=A0= =C2=A0 if (speed >=3D 57600)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return 5760= 0;
=C2=A0 =C2=A0 else if (speed >=3D 38400)
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 return 38400;
=C2=A0 =C2=A0 else if (speed >=3D 19200)
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 return 19200;
=C2=A0 =C2=A0 else if (speed >= =3D 9600)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return 9600;
=C2=A0 =C2=A0 else= if (speed >=3D 4800)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return 4800;
=C2= =A0 =C2=A0 else if (speed >=3D 2400)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 retu= rn 2400;
=C2=A0 =C2=A0 else
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return 1200;<= br>}
=C2=A0
Maybe we should check what the BSDs or any other implementations do
here...

Jeff
--0000000000000095f40615d5dcec--