[PATCH v5] PCI: imx6: fix resource leaks in probe error paths

[PATCH v5] PCI: imx6: fix resource leaks in probe error paths

[Zhijian Han]

imx_pcie_probe() leaks both pwrctrl devices and power domains on failure:

- imx_pcie_attach_pd() attaches the "pcie" and "pcie_phy" power domains
  and adds device links to them, but nothing detaches the domains on
  probe failure or deferral, so they leak.

- A failure of devm_pm_runtime_set_active_enabled() returns directly
  without destroying the pwrctrl devices.

- A partial failure inside imx_pcie_attach_pd() leaks the power domains
  that were already attached.

Add imx_pcie_detach_pd() to detach the power domains in reverse order of
acquisition and register it with devm_add_action_or_reset() so the power
domains are released automatically on probe failure.  Add
DL_FLAG_AUTOREMOVE_CONSUMER to the device links so the driver core
removes them automatically when probe fails, instead of tracking and
deleting them manually.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/
Fixes: 2c5768344f88 ("PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()")
Signed-off-by: Zhijian Han <hanzhijian1991@gmail.com>
---
Changes in v5:
- Register imx_pcie_detach_pd() with devm_add_action_or_reset() instead
  of calling it manually on the error paths

Changes in v4:
- Use DL_FLAG_AUTOREMOVE_CONSUMER so the driver core removes the device
  links automatically, instead of tracking and deleting them manually
- Add a Fixes tag

 drivers/pci/controller/dwc/pci-imx6.c | 41 ++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b98d..ec8339e03d58 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -639,6 +639,23 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
 }
 #endif
 
+static void imx_pcie_detach_pd(struct imx_pcie *imx_pcie)
+{
+	if (!IS_ERR_OR_NULL(imx_pcie->pd_pcie_phy)) {
+		dev_pm_domain_detach(imx_pcie->pd_pcie_phy, true);
+		imx_pcie->pd_pcie_phy = NULL;
+	}
+	if (!IS_ERR_OR_NULL(imx_pcie->pd_pcie)) {
+		dev_pm_domain_detach(imx_pcie->pd_pcie, true);
+		imx_pcie->pd_pcie = NULL;
+	}
+}
+
+static void imx_pcie_detach_pd_action(void *data)
+{
+	imx_pcie_detach_pd(data);
+}
+
 static int imx_pcie_attach_pd(struct device *dev)
 {
 	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
@@ -655,24 +672,30 @@ static int imx_pcie_attach_pd(struct device *dev)
 	if (!imx_pcie->pd_pcie)
 		return 0;
 	link = device_link_add(dev, imx_pcie->pd_pcie,
-			DL_FLAG_STATELESS |
 			DL_FLAG_PM_RUNTIME |
-			DL_FLAG_RPM_ACTIVE);
+			DL_FLAG_RPM_ACTIVE |
+			DL_FLAG_AUTOREMOVE_CONSUMER);
 	if (!link) {
 		dev_err(dev, "Failed to add device_link to pcie pd\n");
+		imx_pcie_detach_pd(imx_pcie);
 		return -EINVAL;
 	}
 
 	imx_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy");
-	if (IS_ERR(imx_pcie->pd_pcie_phy))
-		return PTR_ERR(imx_pcie->pd_pcie_phy);
+	if (IS_ERR(imx_pcie->pd_pcie_phy)) {
+		int ret = PTR_ERR(imx_pcie->pd_pcie_phy);
+
+		imx_pcie_detach_pd(imx_pcie);
+		return ret;
+	}
 
 	link = device_link_add(dev, imx_pcie->pd_pcie_phy,
-			DL_FLAG_STATELESS |
 			DL_FLAG_PM_RUNTIME |
-			DL_FLAG_RPM_ACTIVE);
+			DL_FLAG_RPM_ACTIVE |
+			DL_FLAG_AUTOREMOVE_CONSUMER);
 	if (!link) {
 		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
+		imx_pcie_detach_pd(imx_pcie);
 		return -EINVAL;
 	}
 
@@ -1955,6 +1978,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, imx_pcie_detach_pd_action, imx_pcie);
+	if (ret)
+		return ret;
+
 	ret = pci_pwrctrl_create_devices(dev);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
@@ -1975,7 +2002,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 			pm_runtime_no_callbacks(dev);
 			ret = devm_pm_runtime_set_active_enabled(dev);
 			if (ret < 0)
-				return ret;
+				goto err_pwrctrl_destroy;
 		}
 
 		if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_SKIP_L23_READY))

Re: [PATCH v5] PCI: imx6: fix resource leaks in probe error paths

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing DL_FLAG_STATELESS with DL_FLAG_AUTOREMOVE_CONSUMER for device links to virtual PM domain devices creates managed links that violate the driver core state machine, triggering a WARN_ON during successful probe.
--

commit 36162fbf1662cbc9312c01c7d2c2113d6c77e624
Author: Zhijian Han <hanzhijian1991@gmail.com>

PCI: imx6: fix resource leaks in probe error paths

This commit fixes resource leaks involving pwrctrl devices and power domains
in the imx_pcie_probe() error paths. It adds a devm action to detach the
power domains and replaces the stateless device link flag with an autoremove
consumer flag so links are cleaned up automatically.

Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..ec8339e03d584 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -655,24 +672,30 @@ static int imx_pcie_attach_pd(struct device *dev)
>  	if (!imx_pcie->pd_pcie)
>  		return 0;
>  	link = device_link_add(dev, imx_pcie->pd_pcie,
> -			DL_FLAG_STATELESS |
>  			DL_FLAG_PM_RUNTIME |
> -			DL_FLAG_RPM_ACTIVE);
> +			DL_FLAG_RPM_ACTIVE |
> +			DL_FLAG_AUTOREMOVE_CONSUMER);

[Severity: High]
Does changing this link from stateless to managed violate the driver core
state machine for virtual PM domain devices?

Virtual PM domain devices created on the genpd bus never bind to drivers,
so their driver status remains DL_DEV_NO_DRIVER. Because of this,
device_link_add() will initialize a managed link to DL_STATE_DORMANT.

When the consumer device successfully completes its probe, the driver core
calls device_links_driver_bound(). That function iterates over all managed
supplier links and evaluates:

WARN_ON(link->status != DL_STATE_CONSUMER_PROBE)

Since this link is dormant, won't this trigger the WARN_ON and turn a
successful normal boot sequence into a kernel panic on systems with
panic_on_warn enabled?

>  	if (!link) {
>  		dev_err(dev, "Failed to add device_link to pcie pd\n");
> +		imx_pcie_detach_pd(imx_pcie);
>  		return -EINVAL;
>  	}
>  
>  	imx_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy");
> -	if (IS_ERR(imx_pcie->pd_pcie_phy))
> -		return PTR_ERR(imx_pcie->pd_pcie_phy);
> +	if (IS_ERR(imx_pcie->pd_pcie_phy)) {
> +		int ret = PTR_ERR(imx_pcie->pd_pcie_phy);
> +
> +		imx_pcie_detach_pd(imx_pcie);
> +		return ret;
> +	}
>  
>  	link = device_link_add(dev, imx_pcie->pd_pcie_phy,
> -			DL_FLAG_STATELESS |
>  			DL_FLAG_PM_RUNTIME |
> -			DL_FLAG_RPM_ACTIVE);
> +			DL_FLAG_RPM_ACTIVE |
> +			DL_FLAG_AUTOREMOVE_CONSUMER);

[Severity: High]
This has the same issue as the pcie power domain link creation above.

Since DL_FLAG_AUTOREMOVE_CONSUMER implies a managed link, should these
links for virtual PM domain devices remain DL_FLAG_STATELESS to avoid
the WARN_ON during device_links_driver_bound()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902164313.2901302-1-hanzhijian1991@gmail.com?part=1