Re: [PATCH v3 2/4] serial: fix ioctl hangup race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] uart_set_ldisc misses the tty_io_error(tty) check while holding port->mutex, allowing hardware access after the port has been shut down.
--

commit 3a1dbe6dd850d3b9aa0ddbed8997abbaa897c8af
Author: Johan Hovold <johan@kernel.org>

    serial: fix ioctl hangup race
    
    The tty ioctls can race with hangup and end up calling into a tty
    driver for a device that is already gone or powered down.
    
    Add the missing checks to make sure the port has not been hung up before
    accessing the hardware to avoid issues like kernel panic due to
    unclocked accesses.

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 1553bc6cbe7ba..4213fa3dc988b 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c

[ ... ]

> @@ -1647,7 +1658,7 @@ static void uart_set_termios(struct tty_struct *tty,
>  	guard(mutex)(&state->port.mutex);
>  
>  	uport = uart_port_check(state);
> -	if (!uport)
> +	if (!uport || tty_io_error(tty))
>  		return;
>  
>  	/*

[Severity: High]
This isn't a bug introduced by this patch, but should a similar check be
added to uart_set_ldisc?

uart_set_ldisc() also acquires port->mutex and checks if the port is valid,
but it does not check tty_io_error(tty) before calling uport->ops->set_ldisc:

static void uart_set_ldisc(struct tty_struct *tty)
{
        ...
        guard(mutex)(&state->port.mutex);
        uport = uart_port_check(state);
        if (uport && uport->ops->set_ldisc)
                uport->ops->set_ldisc(uport, &tty->termios);
}

If a privileged user executes TIOCSERCONFIG concurrently with a TIOCSETD ioctl,
uart_do_autoconfig() can acquire the mutex, call uart_shutdown() to shut down
the hardware, and set TTY_IO_ERROR.  If uart_set_ldisc() then acquires the
mutex, could it invoke the uport->ops->set_ldisc hardware callback on a
powered-down device since it doesn't verify tty_io_error(tty)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907064418.92953-1-johan@kernel.org?part=2