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

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

[Ali Tariq]

plda_init_interrupts() initializes IRQ domains and creates IRQ mapping but
does not unwind them when later step fails.

If platform_get_irq() or either irq_create_mapping() fails
in plda_init_interrupts(), the domains are never deinitialized. If
irq_create_mapping() fails, port->intx_irq stays initialized.

Add shared error handling labels for error paths to deinitialize IRQ
mappings and IRQ domains.

This issue was found by automated review of sashiko-bot

Fixes: 4602c370bdf6 ("PCI: microchip: Move IRQ functions to pcie-plda-host.c")
Fixes: 76c911396807 ("PCI: plda: Add host init/deinit and map bus functions")
Link: https://lore.kernel.org/linux-pci/20260718120701.DF4111F000E9@smtp.kernel.org/
Signed-off-by: Ali Tariq <alitariq45892@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/pci/controller/plda/pcie-plda-host.c | 30 +++++++++++++++-----
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
index cda8fdb088fd..a2d7079aec73 100644
--- a/drivers/pci/controller/plda/pcie-plda-host.c
+++ b/drivers/pci/controller/plda/pcie-plda-host.c
@@ -419,6 +419,8 @@ static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port)
 	return plda_allocate_msi_domains(port);
 }
 
+static void plda_pcie_irq_domain_deinit(struct plda_pcie_rp *pcie);
+
 int plda_init_interrupts(struct platform_device *pdev,
 			 struct plda_pcie_rp *port,
 			 const struct plda_event *event)
@@ -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;
 		}
 
 		if (event->request_event_irq)
@@ -459,7 +464,7 @@ int plda_init_interrupts(struct platform_device *pdev,
 
 		if (ret) {
 			dev_err(dev, "failed to request IRQ %d\n", event_irq);
-			return ret;
+			goto err_irq_domain_deinit;
 		}
 	}
 
@@ -467,7 +472,8 @@ int plda_init_interrupts(struct platform_device *pdev,
 					    event->intx_event);
 	if (!port->intx_irq) {
 		dev_err(dev, "failed to map INTx interrupt\n");
-		return -ENXIO;
+		ret = -ENXIO;
+		goto err_irq_domain_deinit;
 	}
 
 	/* Plug the INTx chained handler */
@@ -475,8 +481,11 @@ int plda_init_interrupts(struct platform_device *pdev,
 
 	port->msi_irq = irq_create_mapping(port->event_domain,
 					   event->msi_event);
-	if (!port->msi_irq)
-		return -ENXIO;
+	if (!port->msi_irq) {
+		dev_err(dev, "failed to map MSI interrupt\n");
+		ret = -ENXIO;
+		goto err_dispose_irq_mapping;
+	}
 
 	/* Plug the MSI chained handler */
 	irq_set_chained_handler_and_data(port->msi_irq, plda_handle_msi, port);
@@ -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;
 }
 EXPORT_SYMBOL_GPL(plda_init_interrupts);
 
-- 
2.34.1

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

[sashiko-bot]

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

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

[Ali Tariq]

plda_init_interrupts() initializes IRQ domains and creates IRQ mapping but
does not unwind them when later step fails.

If platform_get_irq() or either irq_create_mapping() fails
in plda_init_interrupts(), the domains are never deinitialized. If
irq_create_mapping() fails, port->intx_irq stays initialized.

Add shared error handling labels for error paths to deinitialize IRQ
mappings and IRQ domains.

This issue was found by automated review of sashiko-bot

Fixes: 4602c370bdf6 ("PCI: microchip: Move IRQ functions to pcie-plda-host.c")
Fixes: 76c911396807 ("PCI: plda: Add host init/deinit and map bus functions")
Link: https://lore.kernel.org/linux-pci/20260718120701.DF4111F000E9@smtp.kernel.org/
Signed-off-by: Ali Tariq <alitariq45892@gmail.com>
Cc: stable@vger.kernel.org
---
Changes in v2:
- Added PCI subsystem prefix to the subject line, no functional change.
---
 drivers/pci/controller/plda/pcie-plda-host.c | 30 +++++++++++++++-----
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
index cda8fdb088fd..a2d7079aec73 100644
--- a/drivers/pci/controller/plda/pcie-plda-host.c
+++ b/drivers/pci/controller/plda/pcie-plda-host.c
@@ -419,6 +419,8 @@ static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port)
 	return plda_allocate_msi_domains(port);
 }
 
+static void plda_pcie_irq_domain_deinit(struct plda_pcie_rp *pcie);
+
 int plda_init_interrupts(struct platform_device *pdev,
 			 struct plda_pcie_rp *port,
 			 const struct plda_event *event)
@@ -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;
 		}
 
 		if (event->request_event_irq)
@@ -459,7 +464,7 @@ int plda_init_interrupts(struct platform_device *pdev,
 
 		if (ret) {
 			dev_err(dev, "failed to request IRQ %d\n", event_irq);
-			return ret;
+			goto err_irq_domain_deinit;
 		}
 	}
 
@@ -467,7 +472,8 @@ int plda_init_interrupts(struct platform_device *pdev,
 					    event->intx_event);
 	if (!port->intx_irq) {
 		dev_err(dev, "failed to map INTx interrupt\n");
-		return -ENXIO;
+		ret = -ENXIO;
+		goto err_irq_domain_deinit;
 	}
 
 	/* Plug the INTx chained handler */
@@ -475,8 +481,11 @@ int plda_init_interrupts(struct platform_device *pdev,
 
 	port->msi_irq = irq_create_mapping(port->event_domain,
 					   event->msi_event);
-	if (!port->msi_irq)
-		return -ENXIO;
+	if (!port->msi_irq) {
+		dev_err(dev, "failed to map MSI interrupt\n");
+		ret = -ENXIO;
+		goto err_dispose_irq_mapping;
+	}
 
 	/* Plug the MSI chained handler */
 	irq_set_chained_handler_and_data(port->msi_irq, plda_handle_msi, port);
@@ -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;
 }
 EXPORT_SYMBOL_GPL(plda_init_interrupts);
 
-- 
2.34.1

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

[sashiko-bot]

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