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

[sashiko-bot@kernel.org]

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