Re: [PATCH] PCI: imx6: fix endpoint init error handling

[sashiko-bot@kernel.org]

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

New issues:
- [High] Manual hardware power-off races with devres unregistration, risking a synchronous external abort.

Pre-existing issues:
- [High] A preexisting error path bug causes dw_pcie_edma_remove() to be called twice when dw_pcie_ep_init_registers() fails.
--

--- Patch [1]: [PATCH] PCI: imx6: fix endpoint init error handling ---
commit 4bb2b4d6db6396f6e99b53eb9afa966615f05042
Author: Zhijian Han <hanzhijian1991@gmail.com>

PCI: imx6: fix endpoint init error handling

imx_add_pcie_ep() ignores the return value of imx_pcie_host_init(), so
a failure to enable the clocks, regulators, or PHY during endpoint
initialization goes unnoticed and the driver proceeds to initialize the
endpoint against unpowered hardware.

Link: https://lore.kernel.org/all/20260824152857.DB9771F00A3D@smtp.kernel.org/

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..1dad93e9dd97c 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[ ... ]
> @@ -1654,12 +1657,16 @@ static int imx_add_pcie_ep(struct imx_pcie *imx_pcie,
>  	if (ret) {
>  		dev_err(dev, "Failed to initialize DWC endpoint registers\n");
>  		dw_pcie_ep_deinit(ep);

[Severity: High]
This is a pre-existing issue, but won't this trigger a double-free on the
error path?

If dw_pcie_ep_init_registers() fails, it cleans up EDMA internally:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_init_registers() {
    ...
err_remove_edma:
    dw_pcie_edma_remove(pci);
    return ret;
}

Then this caller in imx_add_pcie_ep() unconditionally calls
dw_pcie_ep_deinit(), which eventually calls dw_pcie_ep_cleanup() and tries to
remove EDMA again:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_cleanup() {
    ...
    dw_pcie_edma_remove(pci);
}

Since dw_edma_remove() in drivers/dma/dw-edma/dw-edma-core.c does not nullify
internal pointers after freeing resources, will executing it twice lead to list
corruption and kernel panics?

> -		return ret;
> +		goto err_host_exit;
>  	}
>  
>  	pci_epc_init_notify(ep->epc);
>  
>  	return 0;
> +
> +err_host_exit:
> +	imx_pcie_host_exit(pp);

[Severity: High]
Can calling imx_pcie_host_exit() directly here cause a synchronous external
abort?

By invoking this function on the error path, clocks and power are severed
while the EPC device might still be registered in sysfs via devres. If
userspace accesses the device while the hardware is powered off, it could
crash.

Should this cleanup be deferred via devm_add_action_or_reset() instead, to
ensure devres handles unregistration before the power and clocks are removed?

> +	return ret;
>  }

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