Re: [PATCH v2] serial: core: shut down initialized port on removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] serial: PM state leak in serial_core_remove_one_port() due to missed uart_change_pm()
--

commit 9122ece37c80d138a14cda352b9aec7db9935327
Author: Igor Putko <igorpetindev@gmail.com>

serial: core: shut down initialized port on removal

This commit clears port->console under the port mutex and explicitly calls
uart_shutdown() if the port is initialized during removal. It aims to fix a
use-after-free by ensuring the port is shut down and its IRQ freed when
unbinding a device that was previously used as a console.

Closes: https://syzkaller.appspot.com/bug?extid=843bf2f48f4d12e6682e
Reported-by: syzbot+843bf2f48f4d12e6682e@syzkaller.appspotmail.com

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 319a4b427f3a3..05450529871e0 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c

[ ... ]

> @@ -3239,6 +3239,13 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
>  	if (uart_console(uport))
>  		unregister_console(uport->cons);
>  
> +	scoped_guard(mutex, &port->mutex) {
> +		port->console = false;
> +
> +		if (tty_port_initialized(port))
> +			uart_shutdown(NULL, state);

[Severity: Medium]
Does calling uart_shutdown() directly here skip the complete TTY port
shutdown sequence?

When unbinding a serial device that was previously opened and closed as a
console, uart_shutdown() only executes a partial teardown. It skips
uport->ops->stop_rx() and transitioning the PM state to OFF, which normally
happen in uart_tty_port_shutdown():

drivers/tty/serial/serial_core.c:uart_tty_port_shutdown() {
        ...
        scoped_guard(uart_port_lock_irq, uport)
                uport->ops->stop_rx(uport);

        serial_base_port_shutdown(uport);
        uart_port_shutdown(port);
        ...
        uart_change_pm(state, UART_PM_STATE_OFF);
}

Could this leave the device in an active power state (UART_PM_STATE_ON)
indefinitely or leave the hardware receiving data and interrupts? Calling
tty_port_shutdown_locked() would execute the complete sequence.

> +	}
> +
>  	/*
>  	 * Free the port IO and memory resources, if any.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003163337.45577-1-igorpetindev@gmail.com?part=1