Re: [PATCH] PCI: starfive: Fix resource leaks on error paths in host_init()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-after-free of IRQ domains during driver unbind or when probe fails after interrupt initialization.
--

--- Patch [1]: [PATCH] PCI: starfive: Fix resource leaks on error paths in host_init() ---
commit 64603196409722c49e238cd53d9d6a6b7efd0b22
Author: Ali Tariq <alitariq45892@gmail.com>

PCI: starfive: Fix resource leaks on error paths in host_init()

This commit unwinds previously acquired PHY and clock/reset resources in
starfive_pcie_host_init() when subsequent initialization steps or regulator
enablement fails.

> diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
> index 298036c3e7f94..86b248a1ea758 100644
> --- a/drivers/pci/controller/plda/pcie-starfive.c
> +++ b/drivers/pci/controller/plda/pcie-starfive.c
[ ... ]
> @@ -303,13 +303,19 @@ static int starfive_pcie_host_init(struct plda_pcie_rp *plda)
>  			   STG_SYSCON_CLKREQ, STG_SYSCON_CLKREQ);
>  
>  	ret = starfive_pcie_clk_rst_init(pcie);
> -	if (ret)
> +	if (ret) {
> +		starfive_pcie_disable_phy(pcie);
>  		return ret;
> +	}
>  
>  	if (pcie->vpcie3v3) {
>  		ret = regulator_enable(pcie->vpcie3v3);
> -		if (ret)
> +		if (ret) {
>  			dev_err_probe(dev, ret, "failed to enable vpcie3v3 regulator\n");
> +			starfive_pcie_clk_rst_deinit(pcie);
> +			starfive_pcie_disable_phy(pcie);
> +			return ret;
> +		}
>  	}

[Severity: High]
This is a pre-existing issue, but since we are fixing error paths in the
host initialization, I noticed a potential use-after-free in the related
PLDA host initialization code.

In drivers/pci/controller/plda/pcie-plda-host.c, plda_init_interrupts()
allocates the interrupt using devm_request_irq():

plda_init_interrupts() {
    ...
    ret = devm_request_irq(dev, event_irq,
                           plda_event_handler,
                           0, NULL, port);
    ...
}

However, on device removal or if probe fails, plda_pcie_host_init() calls
plda_pcie_irq_domain_deinit(), which explicitly removes and frees the event
domain:

plda_pcie_host_init() {
    ...
err_probe:
    plda_pcie_irq_domain_deinit(port);
    ...
}

plda_pcie_irq_domain_deinit() {
    ...
    irq_domain_remove(pcie->event_domain);
    ...
}

Because the IRQs are devm-managed, their cleanup is deferred until after the
driver's probe or remove function returns.

When devres eventually calls free_irq(), will the IRQ core attempt to
deactivate the interrupt using the now-freed irq_data->domain pointer?

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