Re: [PATCH v2] PCI: plda: Fix resource leaks on error paths in plda_init_interrupts()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Error paths blindly clear chained handlers for uninitialized IRQ variables, potentially corrupting IRQ 0.
- [High] The patch fails to dispose event IRQ mappings on error paths, causing a resource leak and triggering a kernel WARN_ON during domain removal.
- [High] The error path disposes of the intx_irq mapping before clearing its chained handler.
- [High] Freeing irq_domain while devres-managed IRQs still reference it leads to a Use-After-Free during devres cleanup.
--

--- Patch [1]: [PATCH v2] PCI: plda: Fix resource leaks on error paths in plda_init_interrupts() ---
commit f27fa706e161e83dc0578cd66d0f41f0380cf44a
Author: Ali Tariq <alitariq45892@gmail.com>
Date:   Tue Jul 21 13:15:22 2026 +0000

    PCI: plda: Fix resource leaks on error paths in plda_init_interrupts()

This commit adds shared error handling labels in plda_init_interrupts() to
deinitialize IRQ mappings and IRQ domains when initialization steps fail.

Link: https://lore.kernel.org/linux-pci/20260718120701.DF4111F000E9@smtp.kernel.org/

> diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
> index f9a34f323ad8c..367d685244d05 100644
> --- a/drivers/pci/controller/plda/pcie-plda-host.c
> +++ b/drivers/pci/controller/plda/pcie-plda-host.c
> @@ -440,14 +442,17 @@ int plda_init_interrupts(struct platform_device *pdev,
>  	}
>  
>  	port->irq = platform_get_irq(pdev, 0);
> -	if (port->irq < 0)
> -		return -ENODEV;
> +	if (port->irq < 0) {
> +		ret = -ENODEV;
> +		goto err_irq_domain_deinit;
> +	}

[Severity: High]
If platform_get_irq() fails early, execution jumps to err_irq_domain_deinit.
Since port->msi_irq and port->intx_irq are not yet initialized (value 0),
will plda_pcie_irq_domain_deinit() attempt to clear the chained handler
for IRQ 0?

plda_pcie_irq_domain_deinit() {
        ...
        irq_set_chained_handler_and_data(pcie->msi_irq, NULL, NULL);
        irq_set_chained_handler_and_data(pcie->intx_irq, NULL, NULL);
        ...
}

This might inadvertently corrupt the system timer or other core system 
interrupts assigned to IRQ 0.

[ ... ]
>  		if (event->request_event_irq)
>  			ret = event->request_event_irq(port, event_irq, i);
>  		else
>  			ret = devm_request_irq(dev, event_irq,
>  					       plda_event_handler,
>  					       0, NULL, port);
>  
>  		if (ret) {
>  			dev_err(dev, "failed to request IRQ %d\n", event_irq);
> -			return ret;
> +			goto err_irq_domain_deinit;
>  		}

[Severity: High]
Are the event IRQ mappings leaked when taking this error path?

If a subsequent devm_request_irq() fails, the code jumps to
err_irq_domain_deinit, calling irq_domain_remove(pcie->event_domain).
However, the previously mapped event IRQs are not disposed of first. The
kernel's irq_domain_remove() function requires all mappings to be disposed
and will trigger a WARN_ON if mappings remain in the domain's radix tree.

[Severity: High]
Can this also lead to a use-after-free during devres cleanup?

When the probe function returns an error, devres cleanup will eventually call
devm_irq_release() and free_irq(). The free_irq() function attempts to
deactivate the IRQ, which dereferences irq_data->domain:

free_irq() -> irq_shutdown() -> irq_domain_deactivate_irq()

Since irq_domain_remove(pcie->event_domain) was already called here in the
error path, will the devres cleanup dereference the freed domain structure?

[ ... ]
>  	irq_set_chained_handler_and_data(port->irq, plda_handle_event, port);
>  
>  	return 0;
> +
> +err_dispose_irq_mapping:
> +	irq_dispose_mapping(port->intx_irq);
> +err_irq_domain_deinit:
> +	plda_pcie_irq_domain_deinit(port);

[Severity: High]
Does this error path dispose of the intx_irq mapping before clearing its
chained handler?

When falling through to err_irq_domain_deinit, plda_pcie_irq_domain_deinit()
will call:

        irq_set_chained_handler_and_data(pcie->intx_irq, NULL, NULL);

Since irq_dispose_mapping() frees the VIRQ descriptor and makes the IRQ
number available for reuse, could clearing the chained handler here disrupt
another device that concurrently allocated this IRQ?

> +
> +	return ret;
>  }

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