Re: [PATCH v8 phy-next 01/31] PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Type confusion in `cdns_plat_pcie_shutdown()` and PM callbacks causes an out-of-bounds read and kernel crash during system shutdown or suspend.
- [High] Missing driver `.remove` callback causes severe Use-After-Free and resource leaks upon driver unbind.
--

commit 0bab3a01514eb340ef73b1b934e9dc310bdbfcda
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe()

This commit updates the error path in cdns_plat_pcie_probe() to ensure that
errors from functions like pm_runtime_get_sync() and cdns_pcie_host_setup()
are properly propagated instead of incorrectly returning success.

> --- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> @@ -126,7 +126,7 @@ static int cdns_plat_pcie_probe(struct platform_device *pdev)
>  	while (phy_count--)
>  		device_link_del(cdns_plat_pcie->pcie->link[phy_count]);
>  
> -	return 0;
> +	return ret;
>  }

I noticed a potential issue here that predates this patch.

This driver does not appear to have a .remove callback, nor does it set
.suppress_bind_attrs = true. If the driver is unbound via sysfs, devres will
clean up managed allocations like the host bridge, EPC structs, and
struct cdns_pcie memory.

However, without a remove callback to call cdns_pcie_host_disable(), does the
PCI core subsystem remain unaware that the host bridge was removed?

Could this lead to a use-after-free if the PCI core continues to access the
freed bridge structure and memory-mapped I/O regions?

A related leak of PHY device links was noted as fixed by a later commit in
this series (PCI: Remove device links to PHY), but it appears the missing
root bus teardown remains unfixed.

>  
>  static void cdns_plat_pcie_shutdown(struct platform_device *pdev)

I also noticed another pre-existing issue related to this function.

In cdns_plat_pcie_probe(), the platform device's driver data is set to a
struct cdns_plat_pcie pointer:

cdns_plat_pcie_probe()
    cdns_plat_pcie = devm_kzalloc(dev, sizeof(*cdns_plat_pcie), GFP_KERNEL);
    ...
    platform_set_drvdata(pdev, cdns_plat_pcie);

However, cdns_plat_pcie_shutdown() retrieves this and casts it directly to a
struct cdns_pcie pointer:

cdns_plat_pcie_shutdown()
    struct cdns_pcie *pcie = dev_get_drvdata(dev);
    ...
    cdns_pcie_disable_phy(pcie);

Since struct cdns_plat_pcie is just a wrapper containing a pointer to the
actual struct cdns_pcie, does this incorrect cast cause an out-of-bounds
read when cdns_pcie_disable_phy() attempts to access pcie->phy_count and
iterate over the PHY pointers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505100523.1922388-1-vladimir.oltean@nxp.com?part=1