[v1] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

[PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

Posted by Felix Gu 2 months ago

When dw_pcie_host_init() fails, the code jumps to err_init which
only calls pm_runtime_put() but skips the clean up of port resources.

Fixes: b593c26d081a ("PCI: eswin: Add ESWIN PCIe Root Complex driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/pci/controller/dwc/pcie-eswin.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
index 2845832b3824..0185b55e09e3 100644
--- a/drivers/pci/controller/dwc/pcie-eswin.c
+++ b/drivers/pci/controller/dwc/pcie-eswin.c
@@ -348,7 +348,7 @@ static int eswin_pcie_probe(struct platform_device *pdev)
 	ret = dw_pcie_host_init(&pci->pp);
 	if (ret) {
 		dev_err(dev, "Failed to init host\n");
-		goto err_init;
+		goto err_pm_runtime_put;
 	}
 
 	return 0;
@@ -358,7 +358,6 @@ static int eswin_pcie_probe(struct platform_device *pdev)
 		reset_control_put(port->perst);
 		list_del(&port->list);
 	}
-err_init:
 	pm_runtime_put(dev);
 
 	return ret;

---
base-commit: 936c21068d7ade00325e40d82bfd2f3f29d9f659
change-id: 20260416-eswin-4d09df4e8f48

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

Re: [PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

Posted by Manivannan Sadhasivam 1 month, 4 weeks ago

On Thu, Apr 16, 2026 at 08:30:20PM +0800, Felix Gu wrote:
> When dw_pcie_host_init() fails, the code jumps to err_init which
> only calls pm_runtime_put() but skips the clean up of port resources.
> 
> Fixes: b593c26d081a ("PCI: eswin: Add ESWIN PCIe Root Complex driver")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-eswin.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
> index 2845832b3824..0185b55e09e3 100644
> --- a/drivers/pci/controller/dwc/pcie-eswin.c
> +++ b/drivers/pci/controller/dwc/pcie-eswin.c
> @@ -348,7 +348,7 @@ static int eswin_pcie_probe(struct platform_device *pdev)
>  	ret = dw_pcie_host_init(&pci->pp);
>  	if (ret) {
>  		dev_err(dev, "Failed to init host\n");
> -		goto err_init;
> +		goto err_pm_runtime_put;
>  	}
>  
>  	return 0;
> @@ -358,7 +358,6 @@ static int eswin_pcie_probe(struct platform_device *pdev)
>  		reset_control_put(port->perst);
>  		list_del(&port->list);
>  	}
> -err_init:
>  	pm_runtime_put(dev);

Error path is really messed up. On pm_runtime_get_sync() failure, it should
go to 'err_free_ports' and on dw_pcie_host_init() failure, it should go to
err_pm_runtime_put.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

Posted by Felix Gu 1 month, 4 weeks ago

On Fri, Apr 17, 2026 at 5:35 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> Error path is really messed up. On pm_runtime_get_sync() failure, it should
> go to 'err_free_ports' and on dw_pcie_host_init() failure, it should go to
> err_pm_runtime_put.
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
Hi Mani,
For pm_runtime_get_sync() failure, it still needs a pm_runtime_put().
So it doesn't need two labels here.

Best regards,
Felix Gu

Re: [PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

Posted by Manivannan Sadhasivam 1 month ago

On Fri, Apr 17, 2026 at 06:14:39PM +0800, Felix Gu wrote:
> On Fri, Apr 17, 2026 at 5:35 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > Error path is really messed up. On pm_runtime_get_sync() failure, it should
> > go to 'err_free_ports' and on dw_pcie_host_init() failure, it should go to
> > err_pm_runtime_put.
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்
> Hi Mani,
> For pm_runtime_get_sync() failure, it still needs a pm_runtime_put().
> So it doesn't need two labels here.
> 

Ah, I missed the fact that we need pm_runtime_put() in error path also. But
still, the order should be reverse. First pm_runtime_put() and then
list_for_each_entry_safe().

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

Posted by zhangsenchuan 1 month, 4 weeks ago

> Subject: [PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure
> 
> When dw_pcie_host_init() fails, the code jumps to err_init which
> only calls pm_runtime_put() but skips the clean up of port resources.
> 
> Fixes: b593c26d081a ("PCI: eswin: Add ESWIN PCIe Root Complex driver")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-eswin.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
> index 2845832b3824..0185b55e09e3 100644
> --- a/drivers/pci/controller/dwc/pcie-eswin.c
> +++ b/drivers/pci/controller/dwc/pcie-eswin.c
> @@ -348,7 +348,7 @@ static int eswin_pcie_probe(struct platform_device *pdev)
>  	ret = dw_pcie_host_init(&pci->pp);
>  	if (ret) {
>  		dev_err(dev, "Failed to init host\n");
> -		goto err_init;
> +		goto err_pm_runtime_put;
>  	}
>  
>  	return 0;
> @@ -358,7 +358,6 @@ static int eswin_pcie_probe(struct platform_device *pdev)
>  		reset_control_put(port->perst);
>  		list_del(&port->list);
>  	}
> -err_init:
>  	pm_runtime_put(dev);
>  
>  	return ret;

Thanks for your reminder.
In some cases, resources are indeed unable to be released. For instance, if the 
clk_bulk_prepare_enable function fails to execute, or if some functions 
in dw_pcie_host_init fail to execute, and the port resources are not released.

Kind regards,
Senchuan
> 
> ---
> base-commit: 936c21068d7ade00325e40d82bfd2f3f29d9f659
> change-id: 20260416-eswin-4d09df4e8f48
>

Re: [PATCH] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

Posted by Markus Elfring 1 month, 4 weeks ago

> When dw_pcie_host_init() fails, the code jumps to err_init which
> only calls pm_runtime_put() but skips the clean up of port resources.

Thus use an other label?

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

Regards,
Markus