1. Patchew
  2. linux
  3. View series

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

Zhijian Han posted 1 patch 3 weeks, 1 day ago
There is a newer version of this series
drivers/pci/controller/dwc/pci-imx6.c | 58 ++++++++++++++++++++++-----
1 file changed, 48 insertions(+), 10 deletions(-)
[PATCH v6] PCI: imx6: fix resource leaks in probe error paths
Posted by Zhijian Han 3 weeks, 1 day ago 
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 or
  removes the links 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 remove the device links and detach the power
domains in reverse order of acquisition and register it with
devm_add_action_or_reset() so they are released automatically on probe
failure.

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 v6:
- Keep the device links stateless and remove them with device_link_del()
  in imx_pcie_detach_pd() instead of using DL_FLAG_AUTOREMOVE_CONSUMER,
  which creates managed links to power-domain devices that never bind a
  driver and would trigger a WARN_ON in device_links_driver_bound()

Changes in v5:
- Register imx_pcie_detach_pd() with devm_add_action_or_reset() instead
  of calling it manually on the error paths

 drivers/pci/controller/dwc/pci-imx6.c | 58 ++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b98d..7dd22b4f826a 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -180,8 +180,12 @@ struct imx_pcie {
 	struct imx_lut_data	luts[IMX95_MAX_LUT];
 	/* power domain for pcie */
 	struct device		*pd_pcie;
+	/* device link for pcie power domain */
+	struct device_link	*pd_link;
 	/* power domain for pcie phy */
 	struct device		*pd_pcie_phy;
+	/* device link for pcie phy power domain */
+	struct device_link	*pd_phy_link;
 	struct phy		*phy;
 	const struct imx_pcie_drvdata *drvdata;
 
@@ -639,10 +643,34 @@ 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_phy_link)) {
+		device_link_del(imx_pcie->pd_phy_link);
+		imx_pcie->pd_phy_link = NULL;
+	}
+	if (!IS_ERR_OR_NULL(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_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);
-	struct device_link *link;
 
 	/* Do nothing when in a single power domain */
 	if (dev->pm_domain)
@@ -654,25 +682,31 @@ static int imx_pcie_attach_pd(struct device *dev)
 	/* Do nothing when power domain missing */
 	if (!imx_pcie->pd_pcie)
 		return 0;
-	link = device_link_add(dev, imx_pcie->pd_pcie,
-			DL_FLAG_STATELESS |
+	imx_pcie->pd_link = device_link_add(dev, imx_pcie->pd_pcie,
+					    DL_FLAG_STATELESS |
 			DL_FLAG_PM_RUNTIME |
 			DL_FLAG_RPM_ACTIVE);
-	if (!link) {
+	if (!imx_pcie->pd_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 |
+	imx_pcie->pd_phy_link = device_link_add(dev, imx_pcie->pd_pcie_phy,
+						DL_FLAG_STATELESS |
 			DL_FLAG_PM_RUNTIME |
 			DL_FLAG_RPM_ACTIVE);
-	if (!link) {
+	if (!imx_pcie->pd_phy_link) {
 		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
+		imx_pcie_detach_pd(imx_pcie);
 		return -EINVAL;
 	}
 
@@ -1955,6 +1989,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 +2013,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 v6] PCI: imx6: fix resource leaks in probe error paths
Posted by Frank Li 3 weeks, 1 day ago 
On Fri, Sep 04, 2026 at 12:03:42AM +0800, Zhijian Han wrote:
> 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 or
>   removes the links 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 remove the device links and detach the power
> domains in reverse order of acquisition and register it with
> devm_add_action_or_reset() so they are released automatically on probe
> failure.
>
> 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 v6:
> - Keep the device links stateless and remove them with device_link_del()
>   in imx_pcie_detach_pd() instead of using DL_FLAG_AUTOREMOVE_CONSUMER,
>   which creates managed links to power-domain devices that never bind a
>   driver and would trigger a WARN_ON in device_links_driver_bound()
>
> Changes in v5:
> - Register imx_pcie_detach_pd() with devm_add_action_or_reset() instead
>   of calling it manually on the error paths
>
>  drivers/pci/controller/dwc/pci-imx6.c | 58 ++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98d..7dd22b4f826a 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -180,8 +180,12 @@ struct imx_pcie {
>  	struct imx_lut_data	luts[IMX95_MAX_LUT];
>  	/* power domain for pcie */
>  	struct device		*pd_pcie;
> +	/* device link for pcie power domain */
> +	struct device_link	*pd_link;
>  	/* power domain for pcie phy */
>  	struct device		*pd_pcie_phy;
> +	/* device link for pcie phy power domain */
> +	struct device_link	*pd_phy_link;
>  	struct phy		*phy;
>  	const struct imx_pcie_drvdata *drvdata;
>
> @@ -639,10 +643,34 @@ 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_phy_link)) {
> +		device_link_del(imx_pcie->pd_phy_link);
> +		imx_pcie->pd_phy_link = NULL;
> +	}
> +	if (!IS_ERR_OR_NULL(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_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);
> -	struct device_link *link;
>
>  	/* Do nothing when in a single power domain */
>  	if (dev->pm_domain)
> @@ -654,25 +682,31 @@ static int imx_pcie_attach_pd(struct device *dev)
>  	/* Do nothing when power domain missing */
>  	if (!imx_pcie->pd_pcie)
>  		return 0;
> -	link = device_link_add(dev, imx_pcie->pd_pcie,
> -			DL_FLAG_STATELESS |
> +	imx_pcie->pd_link = device_link_add(dev, imx_pcie->pd_pcie,
> +					    DL_FLAG_STATELESS |
>  			DL_FLAG_PM_RUNTIME |
>  			DL_FLAG_RPM_ACTIVE);
> -	if (!link) {
> +	if (!imx_pcie->pd_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 |
> +	imx_pcie->pd_phy_link = device_link_add(dev, imx_pcie->pd_pcie_phy,
> +						DL_FLAG_STATELESS |
>  			DL_FLAG_PM_RUNTIME |
>  			DL_FLAG_RPM_ACTIVE);
> -	if (!link) {
> +	if (!imx_pcie->pd_phy_link) {
>  		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
> +		imx_pcie_detach_pd(imx_pcie);
>  		return -EINVAL;
>  	}
>
> @@ -1955,6 +1989,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;
> +

because there is safe guide in imx_pcie_detac_pd_action(), if move it before
imx_pcie_attach_pd(),  Needn't call imx_pcie_detach_pd() in error path.

Or use goto in for error handle in imx_pcie_attach_pd() instead of duplicate
call each error branch.

Frank

>  	ret = pci_pwrctrl_create_devices(dev);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
> @@ -1975,7 +2013,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))
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.