[PATCH 0/3] PCI: dra7xx: fix resource leaks in probe error paths

[PATCH 0/3] PCI: dra7xx: fix resource leaks in probe error paths

[Felix Gu]

Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Felix Gu (3):
      PCI: dra7xx: Fix clock enable leak on probe failure
      PCI: dra7xx: Fix device link leak when devm_phy_get() fails
      PCI: dra7xx: Fix device links leak when dra7xx_pcie_enable_phy() fails

 drivers/pci/controller/dwc/pci-dra7xx.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
---
base-commit: 903c1cf6dff9964e71eda98a39e2e5d442050472
change-id: 20260822-dra7xx-39925beb4219

Best regards,
--  
Felix Gu <ustc.gu@gmail.com>

[PATCH 1/3] PCI: dra7xx: Fix clock enable leak on probe failure

[Felix Gu]

dra7xx_pcie_probe() enables the optional clock but never disables it
when probing fails afterwards.

Switch to devm_clk_get_optional_enabled(), which disables the clock
automatically on failure.

Fixes: 5af9405397bf ("PCI: dra7xx: Get an optional clock")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/pci/controller/dwc/pci-dra7xx.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 6ae5b27e27b3..d43253f7dea6 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -732,15 +732,11 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
 	if (!link)
 		return -ENOMEM;
 
-	dra7xx->clk = devm_clk_get_optional(dev, NULL);
+	dra7xx->clk = devm_clk_get_optional_enabled(dev, NULL);
 	if (IS_ERR(dra7xx->clk))
 		return dev_err_probe(dev, PTR_ERR(dra7xx->clk),
 				     "clock request failed");
 
-	ret = clk_prepare_enable(dra7xx->clk);
-	if (ret)
-		return ret;
-
 	for (i = 0; i < phy_count; i++) {
 		snprintf(name, sizeof(name), "pcie-phy%d", i);
 		phy[i] = devm_phy_get(dev, name);

-- 
2.43.0

[PATCH 2/3] PCI: dra7xx: Fix device link leak when devm_phy_get() fails

[Felix Gu]

When devm_phy_get() fails, the code returned immediately and left the
already created device links behind.

Jump to the existing err_link cleanup instead so those links are
deleted on failure.

Fixes: 7a4db656a635 ("PCI: dra7xx: Create functional dependency between PCIe and PHY")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/pci/controller/dwc/pci-dra7xx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index d43253f7dea6..bf32b14436f9 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -740,8 +740,10 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
 	for (i = 0; i < phy_count; i++) {
 		snprintf(name, sizeof(name), "pcie-phy%d", i);
 		phy[i] = devm_phy_get(dev, name);
-		if (IS_ERR(phy[i]))
-			return PTR_ERR(phy[i]);
+		if (IS_ERR(phy[i])) {
+			ret = PTR_ERR(phy[i]);
+			goto err_link;
+		}
 
 		link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS);
 		if (!link[i]) {

-- 
2.43.0

[PATCH 3/3] PCI: dra7xx: Fix device links leak when dra7xx_pcie_enable_phy() fails

[Felix Gu]

When dra7xx_pcie_enable_phy() fails, the code returned immediately and
left the already created device links behind.

Jump to the existing err_link cleanup instead so those links are
deleted on failure.

Fixes: 7a4db656a635 ("PCI: dra7xx: Create functional dependency between PCIe and PHY")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/pci/controller/dwc/pci-dra7xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index bf32b14436f9..5bd9d1df80ad 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -766,7 +766,7 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
 	ret = dra7xx_pcie_enable_phy(dra7xx);
 	if (ret) {
 		dev_err(dev, "failed to enable phy\n");
-		return ret;
+		goto err_link;
 	}
 
 	platform_set_drvdata(pdev, dra7xx);

-- 
2.43.0

Re: [PATCH 2/3] PCI: dra7xx: Fix device link leak when devm_phy_get() fails

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing cleanup for IRQ domain and chained handler on probe failure paths leads to an irq_domain memory leak and a severe Use-After-Free if the interrupt fires after the probe fails.
--

--- Patch [2]: [PATCH 2/3] PCI: dra7xx: Fix device link leak when devm_phy_get() fails ---
commit d4c6de816ffa3fbf9a40c694558f1e94ac337fd8
Author: Felix Gu <ustc.gu@gmail.com>

PCI: dra7xx: Fix device link leak when devm_phy_get() fails
    
When devm_phy_get() fails, the code returned immediately and left the
already created device links behind.

Jump to the existing err_link cleanup instead so those links are
deleted on failure.

> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index d43253f7dea65..bf32b14436f97 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -740,8 +740,10 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
>  	for (i = 0; i < phy_count; i++) {
>  		snprintf(name, sizeof(name), "pcie-phy%d", i);
>  		phy[i] = devm_phy_get(dev, name);
> -		if (IS_ERR(phy[i]))
> -			return PTR_ERR(phy[i]);
> +		if (IS_ERR(phy[i])) {
> +			ret = PTR_ERR(phy[i]);
> +			goto err_link;
> +		}
>  
>  		link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS);
>  		if (!link[i]) {

[Severity: High]
This is a pre-existing issue, but does the error path in dra7xx_pcie_probe()
and dra7xx_add_pcie_port() leak the IRQ domain and leave a chained handler
registered?

If dra7xx_add_pcie_port() fails after dra7xx_pcie_init_irq_domain()
(e.g., during devm_platform_ioremap_resource_byname() or dw_pcie_host_init()),
or if dra7xx_pcie_probe() jumps to err_gpio or err_deinit later on, the
function returns without calling irq_domain_remove(dra7xx->irq_domain) or
irq_set_chained_handler_and_data(pp->irq, NULL, NULL).

Because the chained handler retains a pointer to pp (which is freed by
devres upon probe failure), could this lead to a Use-After-Free if the
interrupt fires later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-dra7xx-v1-0-23e204d5548b@gmail.com?part=2

Re: [PATCH 1/3] PCI: dra7xx: Fix clock enable leak on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing cleanup of custom INTx IRQ domain and chained handlers on probe failure paths.
- [High] Missing `.remove` callback in a loadable module driver leaves hardware active and leaks resources, leading to Use-After-Free.
--

--- Patch [1]: [PATCH 1/3] PCI: dra7xx: Fix clock enable leak on probe failure ---
commit 949827a7f8e6ad6326095148f4f52df1a75fedb1
Author: Felix Gu <ustc.gu@gmail.com>

PCI: dra7xx: Fix clock enable leak on probe failure

dra7xx_pcie_probe() enables the optional clock but never disables it
when probing fails afterwards.

Switch to devm_clk_get_optional_enabled(), which disables the clock
automatically on failure.

Fixes: 5af9405397bf ("PCI: dra7xx: Get an optional clock")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>

> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 6ae5b27e27b32..d43253f7dea65 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -732,15 +732,11 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
>  	if (!link)
>  		return -ENOMEM;
>  
> -	dra7xx->clk = devm_clk_get_optional(dev, NULL);
> +	dra7xx->clk = devm_clk_get_optional_enabled(dev, NULL);
>  	if (IS_ERR(dra7xx->clk))
>  		return dev_err_probe(dev, PTR_ERR(dra7xx->clk),
>  				     "clock request failed");
>  
> -	ret = clk_prepare_enable(dra7xx->clk);
> -	if (ret)
> -		return ret;
> -

[ ... ]

>  	switch (mode) {
>  	case DW_PCIE_RC_TYPE:
>  		if (!IS_ENABLED(CONFIG_PCI_DRA7XX_HOST)) {
>  			ret = -ENODEV;
>  			goto err_gpio;
>  		}
>  
>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>  				   DEVICE_TYPE_RC);
>  
>  		ret = dra7xx_pcie_unaligned_memaccess(dev);
>  		if (ret)
>  			dev_err(dev, "WA for Errata i870 not applied\n");
>  
>  		ret = dra7xx_add_pcie_port(dra7xx, pdev);
>  		if (ret < 0)
>  			goto err_gpio;

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this error path
permanently leak the custom IRQ domain?

If dra7xx_add_pcie_port() fails during its internal dw_pcie_host_init() call,
it returns an error without cleaning up the IRQ domain and chained handler it
just created via dra7xx_pcie_init_irq_domain().

Since the probe error path goes straight to err_gpio:, we skip err_deinit:
and the domain is never removed. Because the pci and dra7xx structures are
devm-allocated, they are automatically freed on probe failure.

If the shared IRQ fires afterwards, won't the dangling chained handler
dereference the freed pointer and cause a panic?

[ ... ]

>  static struct platform_driver dra7xx_pcie_driver = {
>  	.probe = dra7xx_pcie_probe,
>  	.driver = {
>  		.name	= "dra7-pcie",
>  		.of_match_table = of_dra7xx_pcie_match,
>  		.suppress_bind_attrs = true,
>  		.pm	= &dra7xx_pcie_pm_ops,
>  	},
>  	.shutdown = dra7xx_pcie_shutdown,
>  };
>  module_platform_driver(dra7xx_pcie_driver);

[Severity: High]
This is a pre-existing issue, but does this driver need a .remove callback?

This driver can be built as a loadable module, but lacks a .remove callback.
When the module is unloaded, the driver core will unbind the device and
devres will release memory and unmap IO.

However, since there is no explicit teardown, pm_runtime_disable() is
skipped, the PHY is never disabled, dw_pcie_host_deinit() is not called,
and the custom IRQ domain and chained handlers are left active.

Does this guarantee a use-after-free when the chained interrupt handler
later accesses the devres-freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-dra7xx-v1-0-23e204d5548b@gmail.com?part=1

Re: [PATCH 3/3] PCI: dra7xx: Fix device links leak when dra7xx_pcie_enable_phy() fails

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing `.remove` callback in platform driver leads to memory and state leaks on module unload.
- [High] Chained IRQ handler and IRQ domain are not cleaned up on probe failure, causing memory leaks and a Use-After-Free.
- [Medium] Improper asynchronous runtime PM cleanup in probe error paths leaks the active power state.
--

--- Patch [3]: [PATCH 3/3] PCI: dra7xx: Fix device links leak when dra7xx_pcie_enable_phy() fails ---
commit 10c75b44424d11b46ba7f7d99b796469558a6366
Author: Felix Gu <ustc.gu@gmail.com>

PCI: dra7xx: Fix device links leak when dra7xx_pcie_enable_phy() fails

When dra7xx_pcie_enable_phy() fails, the code returned immediately and left
the already created device links behind. This commit fixes the issue by
jumping to the err_link cleanup path instead.

> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index bf32b14436f97..5bd9d1df80ad3 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -766,7 +766,7 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
>  	ret = dra7xx_pcie_enable_phy(dra7xx);
>  	if (ret) {
>  		dev_err(dev, "failed to enable phy\n");
> -		return ret;
> +		goto err_link;
>  	}
>  
>  	platform_set_drvdata(pdev, dra7xx);

[Severity: High]
This is a pre-existing issue, but does the lack of a remove callback in the
platform driver lead to memory and state leaks on module unload?

The platform driver definition lacks a remove callback to pair with manual
state allocations in probe:

drivers/pci/controller/dwc/pci-dra7xx.c:
static struct platform_driver dra7xx_pcie_driver = {
	.probe = dra7xx_pcie_probe,
	...

If the module is unloaded, could all manual allocations and device states
(such as stateless device links, phy power state, and dwc core state) be
leaked since devres ignores manual resource acquisitions?


[Severity: Medium]
This isn't a bug introduced by this patch, but is the asynchronous runtime pm
cleanup in probe error paths handled correctly?

If probe fails after successfully calling pm_runtime_get_sync(), the error
path calls pm_runtime_put() followed immediately by pm_runtime_disable():

drivers/pci/controller/dwc/pci-dra7xx.c:dra7xx_pcie_probe() {
    ...
err_get_sync:
	pm_runtime_put(dev);
	pm_runtime_disable(dev);
    ...
}

pm_runtime_put() schedules an asynchronous suspend request, but the immediate
call to pm_runtime_disable() cancels any pending asynchronous requests. Will
this leave the device and its power domains trapped in the resumed state?
Should pm_runtime_put_sync() be used here instead?


[Severity: High]
This is a pre-existing issue, but are the chained irq handler and irq domain
properly cleaned up on probe failure?

dra7xx_pcie_init_irq_domain() installs the chained handler with pp as data:

drivers/pci/controller/dwc/pci-dra7xx.c:dra7xx_pcie_init_irq_domain() {
    ...
	irq_set_chained_handler_and_data(pp->irq, dra7xx_pcie_msi_irq_handler,
					 pp);
    ...
}

If dw_pcie_host_init() subsequently fails:

drivers/pci/controller/dwc/pci-dra7xx.c:dra7xx_add_pcie_port() {
    ...
	ret = dw_pcie_host_init(pp);
	if (ret) {
		dev_err(dev, "failed to initialize host\n");
		return ret;
	}
    ...
}

The probe error path jumps to err_gpio, bypassing any cleanup for the
chained handler or irq domain. Devres will then free the pci structure
containing pp. If a hardware interrupt fires after this probe failure, could
this result in a use-after-free when the interrupt executes
dra7xx_pcie_msi_irq_handler with the freed pp pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-dra7xx-v1-0-23e204d5548b@gmail.com?part=3

Re: [PATCH 0/3] PCI: dra7xx: fix resource leaks in probe error paths

[Luca Ceresoli]

On Sat Aug 22, 2026 at 5:07 PM CEST, Felix Gu wrote:
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>

All series:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com