Re: [PATCH v4 7/7] 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 8250 port unregistration on probe error paths leads to Use-After-Free.
--

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

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

This commit adds the aspeed,vuart-over-pci property and a syscon phandle to
VUART 3 and 4 to indicate they are VUART over PCI.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index 7c02633f2bd69..2b51749300f87 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>;
> +				syscon = <&syscon>;
>  				no-loopback-test;
> +				aspeed,vuart-over-pci;

[Severity: High]
This is a pre-existing issue, but adding this property exposes a new
failure path in the driver that triggers a use-after-free.

If aspeed_ast2600_vuart_over_pci_set_enabled() fails during
aspeed_vuart_probe(), the code jumps to the err_sysfs_remove label:

drivers/tty/serial/8250/8250_aspeed_vuart.c:aspeed_vuart_probe() {
...
err_sysfs_remove:
        sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
        return rc;
}

This error path returns without calling serial8250_unregister_port() or
timer_delete_sync(). Because the probe failed, the devm_ framework
automatically frees the vuart struct. However, the 8250 serial core still
has the port registered from an earlier call to
serial8250_register_8250_port(), resulting in a dangling pointer.

Could this allow a local user who opens the port to trigger a
use-after-free in aspeed_vuart_startup() when it dereferences vuart->port?

>  				status = "disabled";
>  			};

[ ... ]

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