Re: [PATCH v5 6/6] ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing error cleanup in `aspeed_vuart_probe` leads to a dangling 8250 port and use-after-free on probe failure.
--

--- Patch [6]: [PATCH v5 6/6] ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4 ---
commit 373b41ed9602ed016ca9e86ec139c751e10daac0
Author: Grégoire Layet <gregoire.layet@9elements.com>

ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4

The VUART 3 and 4 are VUART over PCI.
Set this flag to indicate this information.
Also set the syscon phandle.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index 7c02633f2bd69..fbd7ab6332a1e 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> @@ -722,7 +722,9 @@ vuart3: serial@1e787800 {
>  				reg-shift = <2>;
>  				interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&syscon ASPEED_CLK_APB2>;
> +				aspeed,scu-syscon = <&syscon>;
>  				no-loopback-test;
> +				aspeed,vuart-over-pci;

[Severity: High]
This is a pre-existing issue, but does enabling this property expose a missing
error cleanup path in the driver?

If a probe failure occurs in drivers/tty/serial/8250/8250_aspeed_vuart.c
after the port is registered (for example, if setting up the vuart-over-pci
functionality fails), the error path skips unregistering the 8250 port:

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

Does this leave the registered port alive while its private data is freed by
the devres framework?

If the port later handles an interrupt via aspeed_vuart_handle_irq() or is
accessed, could it dereference the freed memory and cause a use-after-free
crash?

Would it be safer to ensure serial8250_unregister_port() is called in this
error path?

>  				status = "disabled";
>  			};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785932211.git.gregoire.layet@9elements.com?part=6