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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unwinding paths call `plda_pcie_irq_domain_deinit()`, which blindly clears chained handlers for uninitialized or explicitly disposed IRQs.
- [High] The newly introduced error paths destroy IRQ domains without first disposing dynamically mapped `event_irq`s, leading to descriptor leaks, kernel `WARN_ON` triggers, and Use-After-Free crashes.
--

--- Patch [1]: [PATCH] Fix resource leaks on error paths in plda_init_interrupts() ---
commit c246e8c073fd516ae7484006a7fd3cf6cd55adf8
Author: Ali Tariq <alitariq45892@gmail.com>

Fix resource leaks on error paths in plda_init_interrupts()

This commit attempts to fix resource leaks in plda_init_interrupts() by
adding shared error handling labels. It aims to deinitialize IRQ domains and
mappings 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;
> +	}
>  
>  	for_each_set_bit(i, &port->events_bitmap, port->num_events) {
>  		event_irq = irq_create_mapping(port->event_domain, i);
>  		if (!event_irq) {
>  			dev_err(dev, "failed to map hwirq %d\n", i);
> -			return -ENXIO;
> +			ret = -ENXIO;
> +			goto err_irq_domain_deinit;
>  		}

[Severity: High]
Are the previously mapped event IRQs properly disposed here?

If irq_create_mapping() fails on a subsequent iteration of this loop, the
error path jumps to err_irq_domain_deinit and removes the domains via
irq_domain_remove().

Because the previously mapped event IRQs are never disposed,
irq_domain_remove() will enforce that the radix tree is empty and trigger a
warning. The undisposed descriptors still reference the freed domain, which
can cause a use-after-free during subsequent cleanup by the IRQ core.

[ ... ]

> @@ -485,6 +494,13 @@ int plda_init_interrupts(struct platform_device *pdev,
>  	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);
> +
> +	return ret;
>  }

[Severity: High]
Could this error path operate on uninitialized or disposed IRQs?

If platform_get_irq() fails earlier in the function, port->msi_irq and
port->intx_irq are still uninitialized (0). The code jumps to
err_irq_domain_deinit and calls plda_pcie_irq_domain_deinit(), which blindly
clears chained handlers without checking if they were set:

plda_pcie_irq_domain_deinit(struct plda_pcie_rp *pcie) {
    irq_set_chained_handler_and_data(pcie->irq, NULL, NULL);
    irq_set_chained_handler_and_data(pcie->msi_irq, NULL, NULL);
    irq_set_chained_handler_and_data(pcie->intx_irq, NULL, NULL);
    ...
}

Operating on IRQ 0 inadvertently disables its handler, which can freeze the
system if it happens to be the system timer.

Additionally, if the MSI interrupt mapping fails, the code falls through
from err_dispose_irq_mapping:

err_dispose_irq_mapping:
    irq_dispose_mapping(port->intx_irq);
err_irq_domain_deinit:
    plda_pcie_irq_domain_deinit(port);

This explicitly disposes port->intx_irq, but then falls through to
plda_pcie_irq_domain_deinit() which attempts to clear the chained handler
using the just-disposed virtual IRQ descriptor.

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