Re: [PATCH v5 4/8] serial: max310x: wait for TX to drain before powering down in shutdown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] serial: max310x: local DoS via uninterruptible sleep in max310x_shutdown()
--

commit 4e0091905a683e3eb7b2b59d98c46ef3bed3cdef
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

serial: max310x: wait for TX to drain before powering down in shutdown

This commit modifies the max310x shutdown sequence to wait for the TX FIFO to
drain before powering down the port. It calculates a per-character duration
based on frame size and baud rate, then polls the TX empty status in a loop
to prevent truncating the final transmitted byte on close.

> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c

[ ... ]

> @@ -1231,6 +1236,20 @@ static int max310x_startup(struct uart_port *port)
>  
>  static void max310x_shutdown(struct uart_port *port)
>  {
> +	struct max310x_one *one = to_max310x_port(port);
> +	unsigned int loops = port->fifosize + 1;
> +
> +	/*
> +	 * The tty layer waits for tx_empty() before close(), but tx_empty()
> +	 * only reflects the chip TX FIFO - the last character may still be in
> +	 * the transmit shift register. Let the FIFO drain and the final
> +	 * character clock out before the port is powered down, otherwise
> +	 * close() truncates the last byte on the wire.
> +	 */
> +	while (!max310x_tx_empty(port) && loops-- > 0)
> +		fsleep(one->one_char_duration_us);

[Severity: High]
Can this loop trigger a local denial of service or a hung task panic?

If a user configures a very low baud rate (e.g., 24 baud or lower), fills the
hardware TX FIFO, stops the receiver from reading (e.g., via hardware flow
control CTS), and closes the port, they can hit Ctrl-C to abort the TTY core's
interruptible uart_wait_until_sent() call. The execution path then proceeds
directly to max310x_shutdown().

Because this loop ignores signals and waits in uninterruptible sleep, bypassing
TTY layer timeouts, it forces the kernel to wait strictly on the baud rate. At
24 baud, one character takes ~416 ms, leading to ~53 seconds (129 iterations *
416 ms) of uninterruptible sleep while holding the port mutex. At 4 baud, the
sleep jumps to ~322 seconds, which immediately triggers the default 120-second
hung task watchdog.

> +	fsleep(one->one_char_duration_us);
> +
>  	/* Disable all interrupts */
>  	max310x_port_write(port, MAX310X_IRQEN_REG, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-max310x-rs485-sw-delay-v5-0-ae46afa583f2@vaisala.com?part=4