Re: [PATCH 2/3] PCI: dra7xx: Fix device link leak when devm_phy_get() fails

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

Pre-existing issues:
- [High] Missing cleanup for IRQ domain and chained handler on probe failure paths leads to an irq_domain memory leak and a severe Use-After-Free if the interrupt fires after the probe fails.
--

--- Patch [2]: [PATCH 2/3] PCI: dra7xx: Fix device link leak when devm_phy_get() fails ---
commit d4c6de816ffa3fbf9a40c694558f1e94ac337fd8
Author: Felix Gu <[email protected]>

PCI: dra7xx: Fix device link leak when devm_phy_get() fails
    
When devm_phy_get() fails, the code returned immediately and left the
already created device links behind.

Jump to the existing err_link cleanup instead so those links are
deleted on failure.

> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index d43253f7dea65..bf32b14436f97 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -740,8 +740,10 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
>  	for (i = 0; i < phy_count; i++) {
>  		snprintf(name, sizeof(name), "pcie-phy%d", i);
>  		phy[i] = devm_phy_get(dev, name);
> -		if (IS_ERR(phy[i]))
> -			return PTR_ERR(phy[i]);
> +		if (IS_ERR(phy[i])) {
> +			ret = PTR_ERR(phy[i]);
> +			goto err_link;
> +		}
>  
>  		link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS);
>  		if (!link[i]) {

[Severity: High]
This is a pre-existing issue, but does the error path in dra7xx_pcie_probe()
and dra7xx_add_pcie_port() leak the IRQ domain and leave a chained handler
registered?

If dra7xx_add_pcie_port() fails after dra7xx_pcie_init_irq_domain()
(e.g., during devm_platform_ioremap_resource_byname() or dw_pcie_host_init()),
or if dra7xx_pcie_probe() jumps to err_gpio or err_deinit later on, the
function returns without calling irq_domain_remove(dra7xx->irq_domain) or
irq_set_chained_handler_and_data(pp->irq, NULL, NULL).

Because the chained handler retains a pointer to pp (which is freed by
devres upon probe failure), could this lead to a Use-After-Free if the
interrupt fires later?

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