[PATCH] PCI: imx6: fix power domain leak on probe failure

[PATCH] PCI: imx6: fix power domain leak on probe failure

[hanzhijian]

imx_pcie_attach_pd() attaches the "pcie" and "pcie_phy" power domains
and creates device links to them, but nothing detaches them when
imx_pcie_probe() subsequently fails or defers.  This leaks the power
domains and device links on every probe failure or deferral.

Save the device links and add imx_pcie_detach_pd() to release them,
calling it from the probe error paths.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/
Signed-off-by: hanzhijian <hanzhijian1991@gmail.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b..4df548679 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -182,6 +182,9 @@ struct imx_pcie {
 	struct device		*pd_pcie;
 	/* power domain for pcie phy */
 	struct device		*pd_pcie_phy;
+	/* device links for the power domains */
+	struct device_link	*pd_link;
+	struct device_link	*pd_phy_link;
 	struct phy		*phy;
 	const struct imx_pcie_drvdata *drvdata;
 
@@ -662,6 +665,7 @@ static int imx_pcie_attach_pd(struct device *dev)
 		dev_err(dev, "Failed to add device_link to pcie pd\n");
 		return -EINVAL;
 	}
+	imx_pcie->pd_link = link;
 
 	imx_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy");
 	if (IS_ERR(imx_pcie->pd_pcie_phy))
@@ -675,10 +679,24 @@ static int imx_pcie_attach_pd(struct device *dev)
 		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
 		return -EINVAL;
 	}
+	imx_pcie->pd_phy_link = link;
 
 	return 0;
 }
 
+static void imx_pcie_detach_pd(struct imx_pcie *imx_pcie)
+{
+	if (imx_pcie->pd_link)
+		device_link_del(imx_pcie->pd_link);
+	if (imx_pcie->pd_pcie)
+		dev_pm_domain_detach(imx_pcie->pd_pcie, true);
+
+	if (imx_pcie->pd_phy_link)
+		device_link_del(imx_pcie->pd_phy_link);
+	if (imx_pcie->pd_pcie_phy)
+		dev_pm_domain_detach(imx_pcie->pd_pcie_phy, true);
+}
+
 static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
 {
 	if (enable)
@@ -1956,8 +1974,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = pci_pwrctrl_create_devices(dev);
-	if (ret)
+	if (ret) {
+		imx_pcie_detach_pd(imx_pcie);
 		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
+	}
 
 	pci->use_parent_dt_ranges = true;
 	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
@@ -1999,6 +2019,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	return 0;
 
 err_pwrctrl_destroy:
+	imx_pcie_detach_pd(imx_pcie);
 	if (ret != -EPROBE_DEFER)
 		pci_pwrctrl_destroy_devices(dev);
 	return ret;
-- 
2.43.0

Re: [PATCH] PCI: imx6: fix power domain leak on probe failure

[sashiko-bot]

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

New issues:
- [High] LIFO order violations during error cleanup cause premature power domain detachment.

Pre-existing issues:
- [High] Resource leak of power domain and device link on partial attach failure.
- [High] Probe error path bypasses cleanup logic, leaking power domains and pwrctrl devices.
--

--- Patch [1]: [PATCH] PCI: imx6: fix power domain leak on probe failure ---
commit 887e76ee860dcc87d2150e367092c569b216fc14
Author: hanzhijian <hanzhijian1991@gmail.com>

PCI: imx6: fix power domain leak on probe failure

This commit fixes a power domain leak in the i.MX6 PCIe driver by
introducing imx_pcie_detach_pd() and calling it in probe error paths.
It also saves the device links to properly release them during cleanup.

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..4df548679464c 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -662,6 +665,7 @@ static int imx_pcie_attach_pd(struct device *dev)
>  		dev_err(dev, "Failed to add device_link to pcie pd\n");
>  		return -EINVAL;
>  	}
> +	imx_pcie->pd_link = link;
>  
>  	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);

[Severity: High]
This is a pre-existing issue, but if dev_pm_domain_attach_by_name() fails
here, does it leak the previously attached "pcie" power domain and its
device link?

Because the function returns directly without cleaning up the earlier
attach, these resources might be permanently leaked on every probe deferral.

Furthermore, the function leaves an ERR_PTR in imx_pcie->pd_pcie_phy,
which would cause a crash if a cleanup path ever attempted to use
imx_pcie_detach_pd() to handle this partial failure.

> @@ -675,10 +679,24 @@ static int imx_pcie_attach_pd(struct device *dev)
>  		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
>  		return -EINVAL;
>  	}
> +	imx_pcie->pd_phy_link = link;
>  
>  	return 0;
>  }
>  
> +static void imx_pcie_detach_pd(struct imx_pcie *imx_pcie)
> +{
> +	if (imx_pcie->pd_link)
> +		device_link_del(imx_pcie->pd_link);
> +	if (imx_pcie->pd_pcie)
> +		dev_pm_domain_detach(imx_pcie->pd_pcie, true);
> +
> +	if (imx_pcie->pd_phy_link)
> +		device_link_del(imx_pcie->pd_phy_link);
> +	if (imx_pcie->pd_pcie_phy)
> +		dev_pm_domain_detach(imx_pcie->pd_pcie_phy, true);
> +}

[Severity: High]
Does imx_pcie_detach_pd() detach the power domains in the wrong order?

The power domains are attached in FIFO order ("pcie", then "pcie_phy") in
imx_pcie_attach_pd(). However, they are also detached in FIFO order here.

Should the "pcie_phy" domain and link be detached before the "pcie" domain
to properly follow reverse initialization (LIFO) order?

[ ... ]

> @@ -1999,6 +2019,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_pwrctrl_destroy:

[Severity: High]
This is a pre-existing issue, but does the probe error path for PM runtime
enablement bypass this cleanup logic?

Looking at imx_pcie_probe() earlier in the function:

drivers/pci/controller/dwc/pci-imx6.c:imx_pcie_probe() {
    ...
	if (imx_pcie->drvdata->flags & IMX_PCIE_FLAG_PM_RUNTIME) {
		pm_runtime_no_callbacks(dev);
		ret = devm_pm_runtime_set_active_enabled(dev);
		if (ret < 0)
			return ret;
	}
    ...
}

If devm_pm_runtime_set_active_enabled() fails, it returns directly. Because
it completely bypasses the err_pwrctrl_destroy label, doesn't it permanently
leak the power domains and pwrctrl devices initialized earlier in the probe?

> +	imx_pcie_detach_pd(imx_pcie);
>  	if (ret != -EPROBE_DEFER)
>  		pci_pwrctrl_destroy_devices(dev);
>  	return ret;

[Severity: High]
Does detaching the power domains before destroying the pwrctrl devices
violate LIFO cleanup order?

If a failure occurs after pci_pwrctrl_create_devices() succeeds, this calls
imx_pcie_detach_pd() to cut power before pci_pwrctrl_destroy_devices() has
a chance to unregister the devices. 

Could this cause the driver remove() callbacks to attempt to access
unpowered PCI registers and trigger a hardware hang or kernel panic?

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

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

[hanzhijian]

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 creates device links to them, but the device links are only kept
  in a local variable and nothing detaches the domains on probe failure
  or deferral, so all of them 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
  and device links that were already attached.

Save the device links, add imx_pcie_detach_pd() to release the power
domains and device links in reverse order of acquisition, call it from
the probe error paths, and make imx_pcie_attach_pd() release everything
it has attached so far on failure.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/
Signed-off-by: hanzhijian <hanzhijian1991@gmail.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 42 ++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b..a2bae4a2c 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -182,6 +182,9 @@ struct imx_pcie {
 	struct device		*pd_pcie;
 	/* power domain for pcie phy */
 	struct device		*pd_pcie_phy;
+	/* device links for the power domains */
+	struct device_link	*pd_link;
+	struct device_link	*pd_phy_link;
 	struct phy		*phy;
 	const struct imx_pcie_drvdata *drvdata;
 
@@ -639,6 +642,26 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
 }
 #endif
 
+static void imx_pcie_detach_pd(struct imx_pcie *imx_pcie)
+{
+	if (imx_pcie->pd_phy_link) {
+		device_link_del(imx_pcie->pd_phy_link);
+		imx_pcie->pd_phy_link = NULL;
+	}
+	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 (imx_pcie->pd_link) {
+		device_link_del(imx_pcie->pd_link);
+		imx_pcie->pd_link = 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 int imx_pcie_attach_pd(struct device *dev)
 {
 	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
@@ -660,12 +683,18 @@ static int imx_pcie_attach_pd(struct device *dev)
 			DL_FLAG_RPM_ACTIVE);
 	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_link = link;
 
 	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 |
@@ -673,8 +702,10 @@ static int imx_pcie_attach_pd(struct device *dev)
 			DL_FLAG_RPM_ACTIVE);
 	if (!link) {
 		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
+		imx_pcie_detach_pd(imx_pcie);
 		return -EINVAL;
 	}
+	imx_pcie->pd_phy_link = link;
 
 	return 0;
 }
@@ -1956,8 +1987,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = pci_pwrctrl_create_devices(dev);
-	if (ret)
+	if (ret) {
+		imx_pcie_detach_pd(imx_pcie);
 		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
+	}
 
 	pci->use_parent_dt_ranges = true;
 	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
@@ -1975,7 +2008,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))
@@ -1999,6 +2032,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	return 0;
 
 err_pwrctrl_destroy:
+	imx_pcie_detach_pd(imx_pcie);
 	if (ret != -EPROBE_DEFER)
 		pci_pwrctrl_destroy_devices(dev);
 	return ret;
-- 
2.43.0

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

[sashiko-bot]

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

New issues:
- [High] The error path in imx_pcie_probe() calls imx_pcie_detach_pd() before pci_pwrctrl_destroy_devices(), violating the reverse-initialization ordering.

Pre-existing issues:
- [High] imx_add_pcie_ep() ignores the return value of imx_pcie_host_init(). If imx_pcie_host_init() fails, it cleans up and returns an error, leaving clocks and power domains disabled. Proceeding with endpoint initialization will cause dw_pcie_ep_init_registers() to access unclocked PCIe DBI registers, leading to a synchronous external abort (kernel panic) on ARM systems. It also leaves imx_pcie_host_exit() uncalled on subsequent error paths.
--

--- Patch [1]: [PATCH v2] PCI: imx6: fix resource leaks in probe error paths ---
commit 73509c01fd7cdd5762cd2dc7b8e74d13150be953
Author: hanzhijian <hanzhijian1991@gmail.com>

PCI: imx6: fix resource leaks in probe error paths

imx_pcie_probe() leaks both pwrctrl devices and power domains on failure.
Save the device links, add imx_pcie_detach_pd() to release the power domains
and device links in reverse order of acquisition, and call it from the probe
error paths.

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..a2bae4a2c95ba 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -1956,8 +1987,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	ret = pci_pwrctrl_create_devices(dev);
> -	if (ret)
> +	if (ret) {
> +		imx_pcie_detach_pd(imx_pcie);
>  		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
> +	}
>  
>  	pci->use_parent_dt_ranges = true;
>  	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {

[Severity: High]
This is a pre-existing issue, but does imx_add_pcie_ep() ignore the return
value of imx_pcie_host_init() when called here?

If imx_pcie_host_init() fails, it cleans up its state, disables clocks and
power, and returns an error code:

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
    ...
    imx_pcie_host_init(pp);
    ep = &pci->ep;
    ...
}

If imx_add_pcie_ep() ignores this error and proceeds with initialization,
will dw_pcie_ep_init_registers() attempt to read from the unclocked PCIe
DBI registers?

Could this trigger a synchronous external abort or kernel panic on ARM
systems?

[ ... ]

> @@ -1999,6 +2032,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_pwrctrl_destroy:
> +	imx_pcie_detach_pd(imx_pcie);
>  	if (ret != -EPROBE_DEFER)
>  		pci_pwrctrl_destroy_devices(dev);
>  	return ret;

[Severity: High]
Does this error path violate the reverse-initialization ordering?

Earlier in the probe function, imx_pcie_attach_pd() attaches power domains,
and then pci_pwrctrl_create_devices() populates child devices.

If probe fails, imx_pcie_detach_pd() is called first, which detaches the
power domains and powers off the hardware. Then pci_pwrctrl_destroy_devices()
is called to remove the child devices.

Could the child drivers' remove callbacks attempt to access the
now-powered-off hardware or rely on the parent's power domain being
active, leading to a system crash or hardware lockup?

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

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

[hanzhijian]

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 creates device links to them, but the device links are only kept
  in a local variable and nothing detaches the domains on probe failure
  or deferral, so all of them 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
  and device links that were already attached.

Save the device links, add imx_pcie_detach_pd() to release the power
domains and device links in reverse order of acquisition, call it from
the probe error paths, and make imx_pcie_attach_pd() release everything
it has attached so far on failure.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/
Signed-off-by: hanzhijian <hanzhijian1991@gmail.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 42 ++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b..467006d83 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -182,6 +182,9 @@ struct imx_pcie {
 	struct device		*pd_pcie;
 	/* power domain for pcie phy */
 	struct device		*pd_pcie_phy;
+	/* device links for the power domains */
+	struct device_link	*pd_link;
+	struct device_link	*pd_phy_link;
 	struct phy		*phy;
 	const struct imx_pcie_drvdata *drvdata;
 
@@ -639,6 +642,26 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
 }
 #endif
 
+static void imx_pcie_detach_pd(struct imx_pcie *imx_pcie)
+{
+	if (imx_pcie->pd_phy_link) {
+		device_link_del(imx_pcie->pd_phy_link);
+		imx_pcie->pd_phy_link = NULL;
+	}
+	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 (imx_pcie->pd_link) {
+		device_link_del(imx_pcie->pd_link);
+		imx_pcie->pd_link = 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 int imx_pcie_attach_pd(struct device *dev)
 {
 	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
@@ -660,12 +683,18 @@ static int imx_pcie_attach_pd(struct device *dev)
 			DL_FLAG_RPM_ACTIVE);
 	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_link = link;
 
 	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 |
@@ -673,8 +702,10 @@ static int imx_pcie_attach_pd(struct device *dev)
 			DL_FLAG_RPM_ACTIVE);
 	if (!link) {
 		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
+		imx_pcie_detach_pd(imx_pcie);
 		return -EINVAL;
 	}
+	imx_pcie->pd_phy_link = link;
 
 	return 0;
 }
@@ -1956,8 +1987,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = pci_pwrctrl_create_devices(dev);
-	if (ret)
+	if (ret) {
+		imx_pcie_detach_pd(imx_pcie);
 		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
+	}
 
 	pci->use_parent_dt_ranges = true;
 	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
@@ -1975,7 +2008,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))
@@ -2001,6 +2034,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 err_pwrctrl_destroy:
 	if (ret != -EPROBE_DEFER)
 		pci_pwrctrl_destroy_devices(dev);
+	imx_pcie_detach_pd(imx_pcie);
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH v3] 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:

Pre-existing issues:
- [High] Endpoint setup failure permanently leaks host resources and ignores initialization errors from imx_pcie_host_init().
--

--- Patch [1]: [PATCH v3] PCI: imx6: fix resource leaks in probe error paths ---
commit a77762d63857f9ca84de44552a329e64c4d4176e
Author: hanzhijian <hanzhijian1991@gmail.com>

PCI: imx6: fix resource leaks in probe error paths

This commit fixes resource leaks in imx_pcie_probe() by introducing
imx_pcie_detach_pd() to properly release power domains and device links
when probing fails. It ensures these resources are cleaned up in reverse
order of acquisition.

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..467006d839a60 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[ ... ]
> @@ -1956,8 +1987,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	ret = pci_pwrctrl_create_devices(dev);
> -	if (ret)
> +	if (ret) {
> +		imx_pcie_detach_pd(imx_pcie);
>  		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
> +	}
>  
>  	pci->use_parent_dt_ranges = true;
>  	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {

[Severity: High]
This is a pre-existing issue, but does imx_add_pcie_ep() leak host resources
on failure, and ignore initialization errors?

Looking at the implementation of imx_add_pcie_ep():

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
	...
	imx_pcie_host_init(pp);
	...
	ret = dw_pcie_ep_init(ep);
	if (ret) {
		dev_err(dev, "failed to initialize endpoint\n");
		return ret;
	}
	...
}

The return value of imx_pcie_host_init() is completely ignored, meaning errors
during clock, regulator, or PHY state initialization aren't caught. If the
hardware fails to power on, the driver proceeds to initialize the endpoint
anyway.

Additionally, if dw_pcie_ep_init() fails, the function returns directly
without calling imx_pcie_host_exit(). Doesn't this bypass teardown and
permanently leak all acquired host resources?

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

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

[Markus Elfring]

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

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

See also:
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34


Would you like to avoid a bit of duplicate source code here?


> Signed-off-by: hanzhijian <hanzhijian1991@gmail.com>

Can an other representation be more desirable for the “personal name”
according to the requirements for the Developer's Certificate of Origin?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n396


> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 42 ++++++++++++++++++++++++---
…

Some contributors would appreciate patch version descriptions.
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n310

Regards,
Markus

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

[Frank Li]

On Mon, Aug 24, 2026 at 03:55:12PM +0800, hanzhijian wrote:

Don't post new version patch at old thread.

>
> 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 creates device links to them, but the device links are only kept
>   in a local variable and nothing detaches the domains on probe failure
>   or deferral, so all of them 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
>   and device links that were already attached.
>
> Save the device links, add imx_pcie_detach_pd() to release the power
> domains and device links in reverse order of acquisition, call it from
> the probe error paths, and make imx_pcie_attach_pd() release everything
> it has attached so far on failure.
>
> Reported-by: sashiko-bot@kernel.org
> Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/
> Signed-off-by: hanzhijian <hanzhijian1991@gmail.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 42 ++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b..467006d83 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -182,6 +182,9 @@ struct imx_pcie {
>         struct device           *pd_pcie;
>         /* power domain for pcie phy */
>         struct device           *pd_pcie_phy;
> +       /* device links for the power domains */
> +       struct device_link      *pd_link;
> +       struct device_link      *pd_phy_link;
>         struct phy              *phy;
>         const struct imx_pcie_drvdata *drvdata;
>
> @@ -639,6 +642,26 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
>  }
>  #endif
>
> +static void imx_pcie_detach_pd(struct imx_pcie *imx_pcie)
> +{
> +       if (imx_pcie->pd_phy_link) {
> +               device_link_del(imx_pcie->pd_phy_link);
> +               imx_pcie->pd_phy_link = NULL;
> +       }
> +       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 (imx_pcie->pd_link) {
> +               device_link_del(imx_pcie->pd_link);
> +               imx_pcie->pd_link = 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 int imx_pcie_attach_pd(struct device *dev)
>  {
>         struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
> @@ -660,12 +683,18 @@ static int imx_pcie_attach_pd(struct device *dev)
>                         DL_FLAG_RPM_ACTIVE);
>         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_link = link;
>
>         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 |

Does DL_FLAG_AUTOREMOVE_CONSUMER work?

Frank

> @@ -673,8 +702,10 @@ static int imx_pcie_attach_pd(struct device *dev)
>                         DL_FLAG_RPM_ACTIVE);
>         if (!link) {
>                 dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
> +               imx_pcie_detach_pd(imx_pcie);
>                 return -EINVAL;
>         }
> +       imx_pcie->pd_phy_link = link;
>
>         return 0;
>  }
> @@ -1956,8 +1987,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
>                 return ret;
>
>         ret = pci_pwrctrl_create_devices(dev);
> -       if (ret)
> +       if (ret) {
> +               imx_pcie_detach_pd(imx_pcie);
>                 return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
> +       }
>
>         pci->use_parent_dt_ranges = true;
>         if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
> @@ -1975,7 +2008,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))
> @@ -2001,6 +2034,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  err_pwrctrl_destroy:
>         if (ret != -EPROBE_DEFER)
>                 pci_pwrctrl_destroy_devices(dev);
> +       imx_pcie_detach_pd(imx_pcie);
>         return ret;
>  }
>
> --
> 2.43.0
>
>