Re: [PATCH v6 5/6] ARM: dts: aspeed: g6: Change vuart compatible string for ast2600

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch series adds a new failure path that makes it more likely. Missing serial8250_unregister_port() and timer_delete_sync() in the error handling path of aspeed_vuart_probe() leads to a use-after-free.
--

--- Patch [5]: [PATCH v6 5/6] ARM: dts: aspeed: g6: Change vuart compatible string for ast2600 ---
commit 0075a2210772d2bff98e062573ee22db87bab4f2
Author: Grégoire Layet <[email protected]>

ARM: dts: aspeed: g6: Change vuart compatible string for ast2600

This commit updates the compatible string for the vuart nodes on aspeed-g6 to
use the ast2600 compatible string. It retains the ast2500 compatible string
as a fallback, allowing the driver to utilize specific ast2600 properties.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index edf4b74f5aba4..c952e2075b4c8 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> @@ -707,7 +707,7 @@ emmc: sdhci@1e750100 {
>  			};
>  
>  			vuart1: serial@1e787000 {
> -				compatible = "aspeed,ast2500-vuart";
> +				compatible = "aspeed,ast2600-vuart", "aspeed,ast2500-vuart";

[Severity: High]
This isn't a bug introduced by this patch, but changing the compatible
string exposes it by enabling a new failure path. Can this regression lead
to a use-after-free during probe failure?

In drivers/tty/serial/8250/8250_aspeed_vuart.c:aspeed_vuart_probe(),
using the "aspeed,ast2600-vuart" compatible string enables new initialization
steps, such as aspeed_ast2600_vuart_over_pci_set_enabled(). If this fails,
it jumps to the err_sysfs_remove label:

err_sysfs_remove:
	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
	return rc;

Since serial8250_register_8250_port() and timer_setup() were called
earlier in the probe function, returning an error without unregistering
the 8250 port or cleaning up the timer will leave the port registered
with a dangling pointer when the devm-allocated vuart structure is freed.
If accessed by the tty layer or userspace, it will trigger an oops or
kernel panic.

Should the error handling block in aspeed_vuart_probe() be updated to
include serial8250_unregister_port() and timer_delete_sync() before
returning?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5