Re: [PATCH 1/3] PCI: cadence: Remove device links to PHY

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The probe function `cdns_plat_pcie_probe` unconditionally returns 0 on failure paths, masking initialization errors from the driver core.
- [High] Shutdown and PM callbacks erroneously cast `struct cdns_plat_pcie *` to `struct cdns_pcie *`, causing severe out-of-bounds memory accesses.
--

commit 93f901fd65f6099ca141c0baa86f19be3649b0d0
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

PCI: cadence: Remove device links to PHY

This patch partially reverts device link pieces from a previous commit
because a PHY consumer driver dereferences fields from struct phy, which
will become impossible. It relies on the PHY core's own device links to
order provider and consumer suspend/resume operations.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> index a1ea24fc3b63f..5900d68c6e83a 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
[ ... ]
> @@ -122,9 +121,6 @@ static int cdns_plat_pcie_probe(struct platform_device *pdev)
>  	pm_runtime_put_sync(dev);
>  	pm_runtime_disable(dev);
>  	cdns_pcie_disable_phy(cdns_plat_pcie->pcie);
> -	phy_count = cdns_plat_pcie->pcie->phy_count;
> -	while (phy_count--)
> -		device_link_del(cdns_plat_pcie->pcie->link[phy_count]);
>  
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does this error path intentionally return 0?

If a setup failure occurs during probe (such as cdns_pcie_host_setup()
failing), the code jumps to this error path, performs partial cleanup,
but explicitly returns 0 instead of propagating the error code.

Since the driver core sees a successful probe, it will keep the device
bound. Will subsequent accesses to the device crash the kernel because
the internal structures and resources are partially freed or disabled?

[Severity: High]
This is another pre-existing issue, but could the platform drvdata setup
cause severe out-of-bounds memory accesses during power management?

The cdns_plat_pcie_probe() function allocates an 8-byte struct cdns_plat_pcie
and assigns it as the platform drvdata. However, cdns_plat_pcie_shutdown(),
cdns_pcie_suspend_noirq(), and cdns_pcie_resume_noirq() retrieve the
drvdata and cast it to the much larger struct cdns_pcie *:

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

Does accessing fields at large offsets, like pcie->phy_count inside
cdns_pcie_disable_phy(), read memory completely out of bounds and
potentially cause an immediate kernel panic during suspend, resume,
or shutdown?

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