Re: [PATCH AUTOSEL 6.18-5.10] serial: 8250: fix possible ISR soft lockup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Silent transmission data loss due to unconditionally clearing the TX FIFO in the generic ISR path.
- [Medium] Time-of-check to time-of-use (TOCTOU) race condition when clearing the RX FIFO leads to dropped incoming characters.
--

commit 0cb544a0f6cdcc5a3f0967c34cc3dc4a541bfc13
Author: Marco Felsch <m.felsch@pengutronix.de>

serial: 8250: fix possible ISR soft lockup

This commit addresses an ISR soft lockup on 8250 UARTs during startup
caused by unhandled FIFO error (FIFOE) interrupts. It prevents the
lockup by clearing the FIFO if a FIFO error is latched but no data is
ready (DR) in the generic IRQ handler.

Link: https://patch.msgid.link/20260519-v7-1-topic-serial-8250-v1-1-56b04293a246@pengutronix.de

> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 719faf92aa8ae..108f8fdbebac6 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1799,6 +1799,13 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
>  
>  	status = serial_lsr_in(up);
>  
> +	/*
> +	 * Recover from no-data-ready and FIFO error condition to avoid getting
> +	 * stuck in the ISR.
> +	 */
> +	if (!(status & UART_LSR_DR) && (status & UART_LSR_FIFOE))
> +		serial8250_clear_and_reinit_fifos(up);

[Severity: High]
Does this code inadvertently cause silent data loss for outgoing
transmissions?

When serial8250_handle_irq() calls serial8250_clear_and_reinit_fifos(),
the helper function resets the hardware FIFOs. Looking at the implementation
of serial8250_clear_fifos(), it explicitly clears the TX FIFO alongside
the RX FIFO:

    serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO |
                   UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);

Any transient line noise or framing error that triggers this recovery path
on a full-duplex connection will indiscriminately wipe the TX FIFO. Because
the kernel has already advanced the transmit ring buffer assuming these bytes
were safely queued in hardware, the flushed bytes are permanently dropped
without error propagation to userspace or the TTY layer. Should the fix
isolate the hardware reset strictly to the RX FIFO by only issuing
UART_FCR_CLEAR_RCVR?

[Severity: Medium]
Can the gap between reading the LSR and resetting the hardware FIFOs create a
time-of-check to time-of-use (TOCTOU) race condition?

The ISR takes a software snapshot of the LSR register via serial_lsr_in()
and evaluates the error condition based on this snapshot. Between reading
the LSR and executing the hardware FIFO reset in
serial8250_clear_and_reinit_fifos(), new valid characters can arrive in
the hardware shift register and enter the RX FIFO.

The unconditional RX FIFO reset will silently flush these newly arrived bytes
before the driver can read them, leading to dropped incoming characters. Is
there a way to prevent newly received characters from being discarded during
this window?

> +
>  	/*
>  	 * If port is stopped and there are no error conditions in the
>  	 * FIFO, then don't drain the FIFO, as this may lead to TTY buffer

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-36-sashal@kernel.org?part=1