linux-kernel - Re: [PATCH 4/5] serial: core, 8250: Add a hook for extra port property reporting

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] [day] [month] [year] [list]

Date:   Sat, 26 Jun 2021 06:12:38 +0200 (CEST)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
cc:     Jiri Slaby <jirislaby@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-mips@...r.kernel.org, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] serial: core, 8250: Add a hook for extra port property
 reporting

On Tue, 15 Jun 2021, Greg Kroah-Hartman wrote:

> > Add a hook for `uart_report_port' to let serial ports report extra 
> > properties beyond `irq' and `base_baud'.  Use it with the 8250 backend 
> > to report extra baud rates supported above the base rate for ports with 
> > the UPF_MAGIC_MULTIPLIER property, so that people have a way to find out 
> > that they are supported with their system, e.g.:
[...]
> Ick, really?  What relies on this print message?  Why do we need a whole
> new uart port hook for this?

 As I noted, perhaps too briefly, in the commit description (see the last 
sentence above) people need to be made aware of the extra baud rates above 
`base_baud' supported, or otherwise they'll have no way to figure out they 
can use them.

 Reporting tweaked `base_baud' would I think cause confusion from the 
inconsistency with the TIOCGSERIAL/TIOCSSERIAL ioctls (e.g. `setserial'), 
and the sysfs flags:

$ cat /sys/class/tty/ttyS[0-2]/flags
0x10010040
0x10010040
0x90000040
$ 

(here from a Malta board) are IMO too obscure for anyone to figure this 
out (bit 16 means the two extra baud rates are supported).

 As explained with 1/5 we could set `base_baud' to 460800 instead and 
hardcode bit 15 of the divisor to 1, relying on undocumented Super I/O IC 
behaviour, but that would require more exhaustive verification than I am 
able to do with just a single board and IC type and revision.

> Isn't there some other way for your specific variant to print out
> another message if you really want to do something "odd" like this?

 There's always another way. :)  How about?

serial8250.0: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
serial8250.0: ttyS0 extra baud rates supported: 230400, 460800

> And you did not document what your new change did anywhere in the tree,
> so people are going to be confused.

 We've been somewhat terse about things, but you're probably right here.  
Sorry about that.

> I've taken the other patches here, but not this one.

 Thanks.  I've posted an alternative printing a message like above, with 
some commentary.  Let me know if that makes you feel more convinced.

  Maciej