Re: [PATCH v9 1/3] PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe()

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

Pre-existing issues:
- [High] The driver lacks a `.remove` callback and does not set `.suppress_bind_attrs = true`, leading to severe resource leaks and a potential use-after-free when unbound via sysfs.
--

--- Patch [1]: [PATCH v9 1/3] PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe() ---
commit 8180a8544f7ea362a9f7505de96e98a724635cba
Author: Vladimir Oltean <[email protected]>

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

This commit modifies the error path of cdns_plat_pcie_probe() to correctly
propagate error codes instead of returning zero. Errors from pm_runtime_get_sync()
and setup functions are now correctly handled.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> index a1ea24fc3b63f..8aed8b4c3e11c 100644
> --- 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;
>  }
>  
>  static void cdns_plat_pcie_shutdown(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but since you are fixing the probe path, could we
also address the missing driver removal handling?

Looking at cdns_plat_pcie_driver:

static struct platform_driver cdns_plat_pcie_driver = {
	.driver = {
		.name = "cdns-pcie",
		.of_match_table = cdns_plat_pcie_of_match,
		.pm = &cdns_pcie_pm_ops,
	},
	.probe = cdns_plat_pcie_probe,
	.shutdown = cdns_plat_pcie_shutdown,
};

The driver lacks a .remove callback and does not set .suppress_bind_attrs = true.

If a user unbinds the driver via sysfs or unloads the module, the platform
driver core will successfully unbind it. The devm subsystem will then
automatically free all managed resources, including cdns_plat_pcie, the host
bridge, and memory mappings.

However, since pci_stop_root_bus() and pci_remove_root_bus() are never explicitly
called, the PCI root bus and its child devices remain registered in the system.

Could any subsequent access to the PCI bus (such as running lspci) dereference
the freed host bridge structure and cause a use-after-free panic?

Would it make sense to set .suppress_bind_attrs = true to prevent unbinding
via sysfs, or implement a proper .remove callback?

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