Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH] PCI: cadence: add missing MODULE_DEVICE_TABLE()
@ 2026-07-04 12:28 Pengpeng Hou
  2026-07-04 12:36 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Pengpeng Hou @ 2026-07-04 12:28 UTC (permalink / raw)
  To: Aksh Garg, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas
  Cc: Pengpeng Hou, linux-pci, linux-kernel

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.

This is a source-level fix.  It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the platform driver, and the missing module alias publication.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/pci/controller/cadence/pcie-cadence-plat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
index b067a3296dd3..a1ea24fc3b63 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 = {


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] PCI: cadence: add missing MODULE_DEVICE_TABLE()
  2026-07-04 12:28 [PATCH] PCI: cadence: add missing MODULE_DEVICE_TABLE() Pengpeng Hou
@ 2026-07-04 12:36 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-04 12:36 UTC (permalink / raw)
  To: Pengpeng Hou; +Cc: linux-pci

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-04 12:36 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-04 12:28 [PATCH] PCI: cadence: add missing MODULE_DEVICE_TABLE() Pengpeng Hou
2026-07-04 12:36 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox