Re: [PATCH] serial: core: Shut down a running console port before removing it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tty: memory leak of xmit_buf in serial_core_remove_one_port for suspended ports
--

commit e63d9623630a1955eeac809c4b0a62f8d1db0bfb
Author: Phil Rosenthal <phil@phil.gs>

serial: core: Shut down a running console port before removing it

This patch shuts down a console port if it is still running when being
removed, ensuring its resources are released. It does this by adding a
conditional call to uart_shutdown() in serial_core_remove_one_port().

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 319a4b427f3a3..104a2b2eec641 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3239,6 +3239,18 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
>  	if (uart_console(uport))
>  		unregister_console(uport->cons);
>  
> +	/*
> +	 * A console port is not shut down on last close (see
> +	 * tty_port_shutdown()), and the hangup above only shuts down a port
> +	 * that has a tty attached. A console that was opened and closed is
> +	 * therefore still running here; shut it down before its resources are
> +	 * released.
> +	 */
> +	mutex_lock(&port->mutex);
> +	if (tty_port_initialized(port))
> +		uart_shutdown(NULL, state);

[Severity: Medium]
Does this code introduce a regression by leaking port->xmit_buf if the serial
device is unbound while suspended?

If a console or normal serial port is suspended (e.g., via system or runtime
PM), uart_suspend_port() clears the initialized flag but leaves
port->xmit_buf allocated:

drivers/tty/serial/serial_core.c:uart_suspend_port() {
        ...
        if (tty_port_initialized(port)) {
                ...
                tty_port_set_suspended(port, true);
                tty_port_set_initialized(port, false);
                ...
}

If the device is then removed or its driver is unbound,
serial_core_remove_one_port() will evaluate tty_port_initialized(port) as
false and skip calling uart_shutdown().

Since uart_shutdown() is the only path in serial_core responsible for
unconditionally calling uart_free_xmit_buf(port) and
serial_base_port_shutdown(uport), skipping it appears to leak the
page-sized transmit buffer.

> +	mutex_unlock(&port->mutex);
> +
>  	/*
>  	 * Free the port IO and memory resources, if any.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-serial-console-remove-v1-1-288b9c315af7@phil.gs?part=1