mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] PCI: imx6: fix resource leaks in probe error paths
@ 2026-08-24 15:04 Zhijian Han
  2026-09-02  7:52 ` Manivannan Sadhasivam
  2026-09-02 16:16 ` Frank Li
  0 siblings, 2 replies; 4+ messages in thread
From: Zhijian Han @ 2026-08-24 15:04 UTC (permalink / raw)
  To: Richard Zhu, Lucas Stach, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
	Frank Li, Sascha Hauer
  Cc: Rob Herring, Pengutronix Kernel Team, Fabio Estevam, linux-pci,
	linux-arm-kernel, imx, linux-kernel, Zhijian Han, sashiko-bot

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 call it from the probe error paths.  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 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 | 37 +++++++++++++++++++++------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b..0b4209365 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -639,6 +639,18 @@ 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 int imx_pcie_attach_pd(struct device *dev)
 {
 	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
@@ -655,24 +667,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;
 	}
 
@@ -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) {
@@ -1975,7 +1995,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 +2021,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


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

* Re: [PATCH v4] PCI: imx6: fix resource leaks in probe error paths
  2026-08-24 15:04 [PATCH v4] PCI: imx6: fix resource leaks in probe error paths Zhijian Han
@ 2026-09-02  7:52 ` Manivannan Sadhasivam
  2026-09-02  8:04   ` Hongxing Zhu
  2026-09-02 16:16 ` Frank Li
  1 sibling, 1 reply; 4+ messages in thread
From: Manivannan Sadhasivam @ 2026-09-02  7:52 UTC (permalink / raw)
  To: Richard Zhu, Frank Li, Zhijian Han
  Cc: Richard Zhu, Lucas Stach, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, Frank Li, Sascha Hauer,
	Rob Herring, Pengutronix Kernel Team, Fabio Estevam, linux-pci,
	linux-arm-kernel, imx, linux-kernel, sashiko-bot

On Mon, Aug 24, 2026 at 11:04:54PM +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 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 call it from the probe error paths.  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>

Frank, Richard, can I get your review for this patch?

- Mani

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

* RE: [PATCH v4] PCI: imx6: fix resource leaks in probe error paths
  2026-09-02  7:52 ` Manivannan Sadhasivam
@ 2026-09-02  8:04   ` Hongxing Zhu
  0 siblings, 0 replies; 4+ messages in thread
From: Hongxing Zhu @ 2026-09-02  8:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Frank Li, Zhijian Han
  Cc: Lucas Stach, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Bjorn Helgaas, Frank Li, Sascha Hauer, Rob Herring,
	Pengutronix Kernel Team, Fabio Estevam, linux-pci,
	linux-arm-kernel, imx, linux-kernel, sashiko-bot


> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: Wednesday, September 2, 2026 3:53 PM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; Frank Li <frank.li@nxp.com>;
> Zhijian Han <hanzhijian1991@gmail.com>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; Lucas Stach
> <l.stach@pengutronix.de>; Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof
> Wilczyński <kwilczynski@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>;
> Frank Li <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Rob
> Herring <robh@kernel.org>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> imx@lists.linux.dev; linux-kernel@vger.kernel.org; sashiko-bot@kernel.org
> Subject: Re: [PATCH v4] PCI: imx6: fix resource leaks in probe error paths
> 
> On Mon, Aug 24, 2026 at 11:04:54PM +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 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 call it from the probe error paths.  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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fall%2F20260822013640.182C01F000E9%40smtp.kernel.org%2
> F&d
> >
> ata=05%7C02%7Chongxing.zhu%40nxp.com%7C8285e77d3eaf45f25a0608df08
> c73e1
> >
> 2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6392393239841792
> 60%7CUn
> >
> known%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwM
> CIsIlAiOi
> >
> JXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=gcw
> b63Hn
> > dHcOrDQnwD37rUN%2F0EUDcnIZOOGrZ7Ylzio%3D&reserved=0
> > Fixes: 2c5768344f88 ("PCI: imx6: Move pci_pwrctrl_create_devices() to
> > imx_pcie_probe()")
> > Signed-off-by: Zhijian Han <hanzhijian1991@gmail.com>
> 
> Frank, Richard, can I get your review for this patch?
Apologies for missing this earlier.
Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>

Best Regards
Richard Zhu
> 
> - Mani

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

* Re: [PATCH v4] PCI: imx6: fix resource leaks in probe error paths
  2026-08-24 15:04 [PATCH v4] PCI: imx6: fix resource leaks in probe error paths Zhijian Han
  2026-09-02  7:52 ` Manivannan Sadhasivam
@ 2026-09-02 16:16 ` Frank Li
  1 sibling, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-09-02 16:16 UTC (permalink / raw)
  To: Zhijian Han
  Cc: Richard Zhu, Lucas Stach, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
	Frank Li, Sascha Hauer, Rob Herring, Pengutronix Kernel Team,
	Fabio Estevam, linux-pci, linux-arm-kernel, imx, linux-kernel,
	sashiko-bot

On Mon, Aug 24, 2026 at 11:04:54PM +0800, Zhijian Han wrote:
> [You don't often get email from hanzhijian1991@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> 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 call it from the probe error paths.  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 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 | 37 +++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b..0b4209365 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -639,6 +639,18 @@ 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 int imx_pcie_attach_pd(struct device *dev)
>  {
>         struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
> @@ -655,24 +667,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;
>         }
>
> @@ -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);

use devm_add_action_or_reset() to do tear down.

Frank

>                 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 +1995,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 +2021,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
>
>

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

end of thread, other threads:[~2026-09-02 16:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-24 15:04 [PATCH v4] PCI: imx6: fix resource leaks in probe error paths Zhijian Han
2026-09-02  7:52 ` Manivannan Sadhasivam
2026-09-02  8:04   ` Hongxing Zhu
2026-09-02 16:16 ` Frank Li

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

all inboxes | Powered by JetHome®