linux-kernel - Re: [PATCH 0/3] serial: Fix support for UPF_SPD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220708132621.4v2es73h52aq3izn@pali>
Date:   Fri, 8 Jul 2022 15:26:21 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Jiri Slaby <jirislaby@...nel.org>,
        Johan Hovold <johan@...nel.org>,
        Marek Behún <kabel@...nel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags

On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
> On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote:
> > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
> > > On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@...nel.org> wrote:
> > > > >
> > > > > Support for UPF_SPD_* flags is currently broken in more drivers for two
> > > > > reasons. First one is that uart_update_timeout() function does not
> > > > 
> > > > the uart_update_timeout()
> > > > 
> > > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that
> > > > > userspace termios structre is modified by most drivers after each
> > > > 
> > > > structure
> > > > 
> > > > ...
> > > > 
> > > > > (error handling was ommited for simplification)
> > > > 
> > > > omitted
> > > > 
> > > > > After calling set_active_spd_cust_baud() function SPD custom divisor
> > > > > should be active and therefore is_spd_cust_active() should return true.
> > > > >
> > > > > But it is not active (cfgetospeed does not return B38400) and this patch
> > > > > series should fix it. I have tested it with 8250 driver.
> > > > 
> > > > drivers
> > > > 
> > > > > Originally Johan Hovold reported that there may be issue with these
> > > > > ASYNC_SPD_FLAGS in email:
> > > > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/
> > > > >
> > > > >
> > > > > Johan, Greg, could you please test these patches if there is not any
> > > > > regression?
> > > > 
> > > > I'm wondering why we are still supporting this ugly hack?
> > > > Doesn't BOTHER work for you?
> > > 
> > > Yes, I too do not want to add more support for these old flags.  If they
> > > have not been working, let's not add support for them as obviously no
> > > one is using them.  Let's try to remove them if at all possible.
> > 
> > Well, it works partially. For more drivers SET method is working, but
> > GET method returns incorrect value. If your userspace application is
> > written in a way that does not retrieve from kernel current settings
> > then it has big probability that application works.
> 
> I do not understand, sorry, what do you mean by this?

I mean that SET methods are working, GET methods not. In this case SET
done via ioctl(TIOCSSERIAL) and GET via ioctl(TIOCGSERIAL).

> And as you are responding to a months-old thread, I am totally lost, and
> don't even know what the patch here was...
> 
> > So, do you really want to remove support for these old flags completely?
> > That would of course break above applications.
> 
> I'm not saying remove them, I'm saying let us not add any more
> dependancies on them in order to keep new applications from ever wanting
> to use them.

Last time you wrote to remove them. Now saying not to remove them. So I
do not understand you now.

> > Note that usage of BOTHER is problematic and in most cases highly
> > impossible if you are using glibc libc.so. BOTHER is incompatible with
> > glibc header files and so you can either include BOTHER/linux termios
> > file (exclusive) OR glibc header files.
> 
> I thought that was all fixed up in newer versions of glibc.  Is that
> really still the case in the latest release?

In which version you though that it was fixed? Last time when I checked
it (half year ago) all these issues were there still present. Now I
briefly grepped glib git source code and seems that nothing was changed,
no support for BOTHER and neither ABI changes. I also looked into
changelong of last 3 released glibc versions there is neither no
information about it.

> > New version of tcsetattr and ioctl_tty manpages would have documented
> > how to use BOTHER (it is currently in the manpages git).
> > 
> > Currently the only known option how to use BOTHER is to completely
> > reimplement all functions from "termios.h", provide custom "termios.h"
> > header file (and not use glibc termios.h nor any file which it includes)
> > and statically link this reimplementation into final application.
> > 
> > So in most cases BOTHER is not alternative to those old SPD flags even
> > for modern applications.
> 
> Using the kernel's .h file should be easy enough here for them, right?

No. Because structures and defines conflicts with glibc structures and
defines. That is why (ABI incompatible) reimplementation is needed.

> And again, I thought glibc was fixed, what about other libc versions?

musl does not support BOTHER too. And I do not know other libc versions
which are used in production.

> thanks,
> 
> greg k-h