mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] making termios BOTHER speeds work
@ 2024-04-11 14:24 Rich Felker
  2024-04-11 16:55 ` Rich Felker
  0 siblings, 1 reply; 6+ messages in thread
From: Rich Felker @ 2024-04-11 14:24 UTC (permalink / raw)
  To: musl

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 in
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 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.

Rich

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

* Re: [musl] making termios BOTHER speeds work
  2024-04-11 14:24 [musl] making termios BOTHER speeds work Rich Felker
@ 2024-04-11 16:55 ` Rich Felker
  2024-04-11 17:29   ` Rich Felker
  0 siblings, 1 reply; 6+ messages in thread
From: Rich Felker @ 2024-04-11 16:55 UTC (permalink / raw)
  To: musl

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 in
> 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 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 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.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,
but these are not useful speeds.

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

Looking at the existing code, it seems like Linux already had separate
input baud encoded in the bits of c_cflag (CIBAUD mask), but we're not
using that. It's not yet clear to me whether these work and whether we
should be using them.

POSIX is also unhelpful/wrong on how to determine if "split baud" is
supported. The RATIONALE for cfgetispeed says:

    Setting the input baud rate to zero was a mechanism to allow for
    split baud rates. Clarifications in this volume of POSIX.1-2017
    have made it possible to determine whether split rates are
    supported and to support them without having to treat zero as a
    special case.

However, tcgetattr specifies:

    If the terminal device supports different input and output baud
    rates, the baud rates stored in the termios structure returned by
    tcgetattr() shall reflect the actual baud rates, even if they are
    equal. If differing baud rates are not supported, the rate
    returned as the output baud rate shall be the actual baud rate. If
    the terminal device does not support split baud rates, the input
    baud rate stored in the termios structure shall be the output rate
    (as one of the symbolic values).

Which clearly gives the *same behavior* in both the case where split
rates are supported but the same, and the case where split rates are
not supported. I have no idea what they intended here.

I think before this can move forward we need to have a better
understanding of what POSIX is supposed to require here and what Linux
actually does.

Rich

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

* Re: [musl] making termios BOTHER speeds work
  2024-04-11 16:55 ` Rich Felker
@ 2024-04-11 17:29   ` Rich Felker
  2024-04-11 17:55     ` Jeffrey Walton
  0 siblings, 1 reply; 6+ messages in thread
From: Rich Felker @ 2024-04-11 17:29 UTC (permalink / raw)
  To: musl

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 in
> > 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 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 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.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,
> 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.

Maybe we should check what the BSDs or any other implementations do
here...

Rich

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

* Re: [musl] making termios BOTHER speeds work
  2024-04-11 17:29   ` Rich Felker
@ 2024-04-11 17:55     ` Jeffrey Walton
  2024-04-11 18:30       ` Markus Wichmann
  2024-04-11 18:34       ` Raphael Kiefmann
  0 siblings, 2 replies; 6+ messages in thread
From: Jeffrey Walton @ 2024-04-11 17:55 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 5769 bytes --]

On Thu, Apr 11, 2024 at 1:30 PM Rich Felker <dalias@libc.org> 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 in
> > > 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 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 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.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,
> > 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 >= 4000000)
        return 4000000;
    else
#endif
#if defined(B3500000)
    if (speed >= 3500000)
        return 3500000;
    else
#endif
...
    if (speed >= 57600)
        return 57600;
    else if (speed >= 38400)
        return 38400;
    else if (speed >= 19200)
        return 19200;
    else if (speed >= 9600)
        return 9600;
    else if (speed >= 4800)
        return 4800;
    else if (speed >= 2400)
        return 2400;
    else
        return 1200;
}


> Maybe we should check what the BSDs or any other implementations do
> here...
>

Jeff

[-- Attachment #2: Type: text/html, Size: 7330 bytes --]

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

* Re: [musl] making termios BOTHER speeds work
  2024-04-11 17:55     ` Jeffrey Walton
@ 2024-04-11 18:30       ` Markus Wichmann
  2024-04-11 18:34       ` Raphael Kiefmann
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Wichmann @ 2024-04-11 18:30 UTC (permalink / raw)
  To: musl

Am Thu, Apr 11, 2024 at 01:55:12PM -0400 schrieb Jeffrey Walton:
> 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.
>

Apparently, slightly mismatched timings are a thing. At work, we have a
certain serial device that works perfectly when connected to a PC at
38400 baud. When connected to one of our embedded devices, though, it
doesn't work. It starts working, if we tell the OS to use 38400 baud,
then hack the divisor register to get something more like 40000 baud.

This is weird, because all other serial devices work perfectly on the
embedded device (both at slower and higher speeds). Just this one box is
very picky about its baud rate.

Ciao,
Markus

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

* Re: [musl] making termios BOTHER speeds work
  2024-04-11 17:55     ` Jeffrey Walton
  2024-04-11 18:30       ` Markus Wichmann
@ 2024-04-11 18:34       ` Raphael Kiefmann
  1 sibling, 0 replies; 6+ messages in thread
From: Raphael Kiefmann @ 2024-04-11 18:34 UTC (permalink / raw)
  To: musl


[-- Attachment #1.1.1: Type: text/plain, Size: 8953 bytes --]

Hi Jeffrey,

I was the one that initiated this in the IRC. There are these cheap 
Chinese mini-PCs that sport an Intel N100 like the T9 Plus [1]. They all 
have a LED ring at the bottom that glows in the stock configuration.

After some search I found out that there is a Windows driver and that 
someone else [1] had a look at the protocol. For whatever reason this 
driver opens a port with a baud rate of 10_000.

It reverse engineered the .NET binary and confirmed that the original 
driver _really_ uses a baud rate of 10_000. I wrote a small CLI 
application that also works on Linux and with a statically linked 
executable I noticed that the application only works from time to time.

I eventually turned to trace to find that the line was missing from the 
execution of a _musl_ based aplication:

ioctl(3, TCSETS2, {c_iflag=IGNPAR, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|, 
c_cflag=BOTHER|CS8|CREAD|HUPCL|CLOCAL, c_lflag=, c_line=N_TTY, 
c_cc=[[VINTR]=0x3, [VQUIT]=0x1c, [VERASE]=0x7f, [VKILL]=0x15, 
[VEOF]=0x4, [VTIME]=0, [VMIN]=0, [VSWTC]=0, [VSTART]=0x11, [VSTOP]=0x13, 
[VSUSP]=0x1a, [VEOL]=0, [VREPRINT]=0x12, [VDISCARD]=0xf, [VWERASE]=0x17, 
[VLNEXT]=0x16, [VEOL2]=0, [17]=0, [18]=0], c_ispeed=10000, c_ospeed=10000})

Instead the following line was present:

ioctl(3, SNDCTL_TMR_START or TCSETS, {c_iflag=IGNPAR, 
c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|, 
c_cflag=BOTHER|CS8|CREAD|HUPCL|CLOCAL, c_lflag=, c_line=N_TTY, 
c_cc=[[VINTR]=0x3, [VQUIT]=0x1c, [VERASE]=0x7f, [VKILL]=0x15, 
[VEOF]=0x4, [VTIME]=0, [VMIN]=0x1, [VSWTC]=0, [VSTART]=0x11, 
[VSTOP]=0x13, [VSUSP]=0x1a, [VEOL]=0, [VREPRINT]=0x12, [VDISCARD]=0xf, 
[VWERASE]=0x17, [VLNEXT]=0x16, [VEOL2]=0, [17]=0, [18]=0]})

I tried to get past my issue with the baud rate by trying a few other 
close baud rates and messing with some pauses, but none of it led to 
persistent results like using an actual baud rate of 10_000.

I've also read about some other cases of weird baud rates, yes they are 
rare, but they exist and especially on platforms that tend to rely on _musl_

Best regards,
Raphael

[1] https://aliexpress.com/item/1005004893120495.html
[2] 
https://old.reddit.com/r/MiniPCs/comments/18icusg/t9_plus_n100_how_to_control_led/

On 11.04.24 19:55, Jeffrey Walton wrote:
> 
> 
> On Thu, Apr 11, 2024 at 1:30 PM Rich Felker <dalias@libc.org 
> <mailto:dalias@libc.org>> 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
>     <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 in
>      > > 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 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 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.html <https://pubs.opengroup.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,
>      > 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 >= 4000000)
>          return 4000000;
>      else
> #endif
> #if defined(B3500000)
>      if (speed >= 3500000)
>          return 3500000;
>      else
> #endif
> ...
>      if (speed >= 57600)
>          return 57600;
>      else if (speed >= 38400)
>          return 38400;
>      else if (speed >= 19200)
>          return 19200;
>      else if (speed >= 9600)
>          return 9600;
>      else if (speed >= 4800)
>          return 4800;
>      else if (speed >= 2400)
>          return 2400;
>      else
>          return 1200;
> }
> 
>     Maybe we should check what the BSDs or any other implementations do
>     here...
> 
> 
> Jeff

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 699 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

end of thread, other threads:[~2024-04-11 18:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-11 14:24 [musl] making termios BOTHER speeds work Rich Felker
2024-04-11 16:55 ` Rich Felker
2024-04-11 17:29   ` Rich Felker
2024-04-11 17:55     ` Jeffrey Walton
2024-04-11 18:30       ` Markus Wichmann
2024-04-11 18:34       ` Raphael Kiefmann

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).