Re: [PATCH v4 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] The patch misses several tty driver callbacks that access hardware and can race with hangup, including those triggered by the TCXONC ioctl and racing writes.
--

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

serial: fix ioctl hangup race

This commit adds tty_io_error checks to several tty ioctls and driver
operations. It aims to ensure the port has not been hung up before accessing
the hardware, avoiding kernel panics.

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 1553bc6cbe7ba..139b938762298 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -896,7 +896,7 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
>  	upf_t old_flags, new_flags;
>  	int retval;
>  
> -	if (!uport)
> +	if (!uport || tty_io_error(tty))
>  		return -EIO;
>  
>  	new_port = new_info->port;

[Severity: High]
This isn't a bug introduced by this patch, but does this miss several other
tty driver callbacks that access hardware and can race with hangup, including
those triggered by the TCXONC ioctl and racing writes?

Invoking hardware callbacks (start_tx, stop_tx, send_xchar) on a powered-down
or unclocked UART port causes bus errors and kernel panics.

Can a user call ioctl(TCXONC) on a hung-up terminal and invoke uart_stop(),
uart_start(), or uart_send_xchar()? These do not check tty_io_error(tty) and
unconditionally access the hardware:

drivers/tty/serial/serial_core.c:uart_stop() {
	port = uart_port_ref_lock(state, &flags);
	if (port)
		port->ops->stop_tx(port);
	...
}

drivers/tty/serial/serial_core.c:uart_start() {
	port = uart_port_ref_lock(state, &flags);
	__uart_start(state);
	...
}

drivers/tty/serial/serial_core.c:uart_send_xchar() {
	if (port->ops->send_xchar)
		port->ops->send_xchar(port, ch);
	...
}

Additionally, can a concurrent write() race with uart_shutdown()? If
uart_write() acquires the port lock after uart_port_shutdown() disables the
hardware but before uart_free_xmit_buf() clears the buffer pointer, it will
pass the !state->port.xmit_buf check and invoke start_tx on the dead hardware:

drivers/tty/serial/serial_core.c:uart_write() {
	port = uart_port_ref_lock(state, &flags);
	if (!state->port.xmit_buf) {
		uart_port_unlock_deref(port, flags);
		return 0;
	}
	...
	__uart_start(state);
	...
}

[ ... ]

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