Re: [PATCH] serial: 8250_aspeed_vuart: shut down the unthrottle timer on removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] serial: 8250: use-after-free and port leaks on probe error
  paths [1]
--

commit afd3348a7cc94ca4f33ad325f21afb53b427217e
Author: Myeonghun Pak <mhun512@gmail.com>

serial: 8250_aspeed_vuart: shut down the unthrottle timer on removal

This commit replaces timer_delete_sync() with timer_shutdown_sync() in
aspeed_vuart_remove() to prevent unthrottle_timer from being re-armed by
an interrupt before the port is unregistered.

> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index 452bef82bec2a..0776675b83823 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -552,7 +552,7 @@ static void aspeed_vuart_remove(struct platform_device *pdev)
>  {
>  	struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
>  
> -	timer_delete_sync(&vuart->unthrottle_timer);
> +	timer_shutdown_sync(&vuart->unthrottle_timer);
>  	aspeed_vuart_set_enabled(vuart, false);
>  	serial8250_unregister_port(vuart->port);
>  	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);

[Severity: Critical]
This problem wasn't introduced by this patch, but does the error path in
aspeed_vuart_probe() leave the port registered [1] and cause a
use-after-free of vuart?

drivers/tty/serial/8250/8250_aspeed_vuart.c:aspeed_vuart_probe() {
	...
	struct uart_8250_port *uport = serial8250_register_8250_port(&port);
	if (IS_ERR(uport)) {
		rc = PTR_ERR(uport);
		goto err_sysfs_remove;
	}
	...
	rc = aspeed_vuart_set_lpc_address(vuart, prop);
	if (rc < 0) {
		dev_err_probe(dev, rc, "invalid value in aspeed,lpc-io-reg property\n");
		goto err_sysfs_remove;
	}
	...
err_sysfs_remove:
	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
	return rc;
}

When aspeed_vuart_set_lpc_address() or a later step fails after
serial8250_register_8250_port(), err_sysfs_remove returns without calling
serial8250_unregister_port().  The driver core then frees the
devm-allocated vuart while the 8250 core still holds port.private_data
pointing to it, so a later interrupt or TTY access dereferences freed
memory.

[1] https://sashiko.dev/#/bug/linux-99bf5468-4b0d-497a-8419-674f926b3cdb

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