Re: [PATCH] PCI: cadence: add missing MODULE_DEVICE_TABLE()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Probe function silently masks initialization failures by returning 0 in its error path.
- [High] Driver data type mismatch leads to out-of-bounds memory access and kernel panic on shutdown and suspend.
--

commit 614cbd9081a5fd803fcc9fe1b172469d540e733c
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

PCI: cadence: add missing MODULE_DEVICE_TABLE()

The driver has an OF match table wired to .of_match_table, but does
not export the table with MODULE_DEVICE_TABLE().

Add the missing MODULE_DEVICE_TABLE(of, ...) entry so module alias
information is generated for OF based module autoloading.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> index b067a3296dd37..a1ea24fc3b63f 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> @@ -163,6 +163,7 @@ static const struct of_device_id cdns_plat_pcie_of_match[] = {
>  	},
>  	{},
>  };
> +MODULE_DEVICE_TABLE(of, cdns_plat_pcie_of_match);
>  
>  static struct platform_driver cdns_plat_pcie_driver = {
>  	.driver = {

[Severity: High]
This isn't a regression introduced by this patch, but I noticed a pre-existing
regression with error handling in the probe function in this file.

In drivers/pci/controller/cadence/pcie-cadence-plat.c:cdns_plat_pcie_probe():

	ret = cdns_pcie_ep_setup(ep);
	if (ret)
		goto err_init;
...
 err_init:
 err_get_sync:
	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;

Should this error path return the actual error code instead of returning 0?

Returning 0 unconditionally masks the probe failure. Does this cause the
driver core to register the device as successfully probed even when PHY or
host/endpoint setup fails, potentially leading to undefined behavior later?

[Severity: High]
This is also a pre-existing regression, but there appears to be a driver data
type mismatch that could lead to out-of-bounds memory access during shutdown
and suspend.

In drivers/pci/controller/cadence/pcie-cadence-plat.c:cdns_plat_pcie_probe(),
the driver data is set to a struct cdns_plat_pcie pointer:

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

However, in cdns_plat_pcie_shutdown(), it is retrieved and cast as a
struct cdns_pcie pointer:

static void cdns_plat_pcie_shutdown(struct platform_device *pdev)
{
	struct device *dev = &pdev->dev;
	struct cdns_pcie *pcie = dev_get_drvdata(dev);
	...
	cdns_pcie_disable_phy(pcie);
}

The same incorrect cast happens in drivers/pci/controller/cadence/pcie-cadence.c
during suspend and resume operations:

static int cdns_pcie_suspend_noirq(struct device *dev)
{
	struct cdns_pcie *pcie = dev_get_drvdata(dev);

	cdns_pcie_disable_phy(pcie);

	return 0;
}

static int cdns_pcie_resume_noirq(struct device *dev)
{
	struct cdns_pcie *pcie = dev_get_drvdata(dev);
	int ret;

	ret = cdns_pcie_enable_phy(pcie);
	...
}

Since struct cdns_plat_pcie is an 8-byte wrapper containing the actual struct
cdns_pcie pointer, does casting it directly to struct cdns_pcie cause
cdns_pcie_disable_phy() and cdns_pcie_enable_phy() to read out-of-bounds
memory when accessing fields like pcie->phy_count and pcie->phy?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704122835.14277-1-pengpeng@iscas.ac.cn?part=1