linux-kernel - Re: [PATCH v1 RESEND] serial: 8250: Set fifo timeout with uart_fifo

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: Thu, 11 Jan 2024 13:38:04 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Michael Pratt <mcpratt@...me>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    LKML <linux-kernel@...r.kernel.org>, 
    linux-serial <linux-serial@...r.kernel.org>, 
    Jiri Slaby <jirislaby@...nel.org>, 
    Wander Lairson Costa <wander@...hat.com>, 
    Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v1 RESEND] serial: 8250: Set fifo timeout with
 uart_fifo_timeout()

On Thu, 11 Jan 2024, Michael Pratt wrote:

> 
> 
> On Thursday, January 11th, 2024 at 01:52, Greg Kroah-Hartman <gregkh@...uxfoundation.org> wrote:
> 
> > On Thu, Jan 11, 2024 at 12:27:07AM +0000, Michael Pratt wrote:
> > 
> > > Commit 8f3631f0f6eb ("serial/8250: Use fifo in 8250 console driver")
> 
> This is the commit that made the issue present itself.
> 
> I'm not sure whether it's right to say that this "fixes" that commit
> since it only caused the issue indirectly, and the diff for this patch
> doesn't touch any lines that the other commit touched, and the method
> I'm using to fix the issue was not available at the time, and also that
> for high baud rates like 115200 everything is still fine...
> (the 10 ms timeout is as old as the tree)
> 
> If that's enough for a "Fixes" tag then go ahead (or tell me to add it),
> but maybe a "Ref" tag would be enough?
> 
> You can see the other thread linked for more discussion on that point if you like...

Besides Fixes tags, you can add dependency information for stable folks 
with Cc tag using the format described in 
Documentation/process/stable-kernel-rules.rst

-- 
 i.


> > > reworked functions for basic 8250 and 16550 type serial devices
> > > in order to enable and use the internal FIFO device for buffering,
> > > however the default timeout of 10 ms remained, which is proving
> > > to be insufficient for low baud rates like 9600, causing data overrun.
> > > 
> > > Unforunately, that commit was written and accepted just before commit
> > > 31f6bd7fad3b ("serial: Store character timing information to uart_port")
> > > which introduced the frame_time member of the uart_port struct
> > > in order to store the amount of time it takes to send one UART frame
> > > relative to the baud rate and other serial port configuration,
> > > and commit f9008285bb69 ("serial: Drop timeout from uart_port")
> > > which established function uart_fifo_timeout() in order to
> > > calculate a reasonable timeout to wait for all frames
> > > in the FIFO device to flush before writing data again
> > > using the now stored frame_time value and size of the buffer.
> > > 
> > > Fix this by using the new function to calculate the timeout
> > > whenever the buffer is larger than 1 byte (unknown port default).
> > > 
> > > Tested on a MIPS device (ar934x) at baud rates 625, 9600, 115200.
> > > 
> > > Signed-off-by: Michael Pratt mcpratt@...me
> > > ---
> > > v1 thread: https://lore.kernel.org/linux-serial/20231125063552.517-1-mcpratt@pm.me/
> > 
> > 
> > What commit id does this fix?
> > 
> > thanks,
> > 
> > greg k-h
> 
> --
> MCP
>