Re: [PATCH 1/2] PCI: dwc: Handle return value from endpoint .init callback

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

Pre-existing issues:
- [High] Double execution of `dw_pcie_edma_remove()` on the `dw_pcie_ep_init_registers()` error path leads to double-free of eDMA IRQs and DMA device unregistration list corruption.
--

--- Patch [1]: [PATCH 1/2] PCI: dwc: Handle return value from endpoint .init callback ---
commit cb2920cc5e4b5269e0e38169339b1b10e422e580
Author: Marek Vasut <[email protected]>

PCI: dwc: Handle return value from endpoint .init callback

Add return value handling for struct dw_pcie_ep_ops .init callback.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 7d2794945704e..d78fb845582b2 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
[ ... ]
> @@ -1367,8 +1367,11 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
>  		list_add_tail(&ep_func->list, &ep->func_list);
>  	}
>  
> -	if (ep->ops->init)
> -		ep->ops->init(ep);
> +	if (ep->ops->init) {
> +		ret = ep->ops->init(ep);
> +		if (ret)
> +			goto err_remove_edma;
> +	}

[Severity: High]
This is a pre-existing issue, but by handling the return value here, this
error path becomes more easily reachable. Can this lead to a double-free
of eDMA IRQs and list corruption?

If ep->ops->init() fails, the code jumps to err_remove_edma which calls
dw_pcie_edma_remove(pci) and returns the error code.

The caller typically catches this error and cleans up again:

drivers/pci/controller/dwc/pci-layerscape-ep.c:ls_pcie_ep_probe() {
    ...
    ret = dw_pcie_ep_init_registers(&pci->ep);
    if (ret) {
        ...
        dw_pcie_ep_deinit(&pci->ep);
        return ret;
    }
}

dw_pcie_ep_deinit() then calls dw_pcie_ep_cleanup(), which unconditionally
executes dw_pcie_edma_remove(pci) a second time.

Since dw_edma_remove() does not clear its internal state (chip->dw = NULL)
after freeing resources, will this second invocation cause a double-free
or kernel oops during a device probe failure?

>  
>  	dw_pcie_ep_disable_bars(ep);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1