Re: [PATCH v3 2/3] PCI: rzg3s: Propagate platform_get_irq_byname() errors

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

Pre-existing issues:
- [High] Missing cleanup of chained IRQ handlers triggers a Use-After-Free panic on probe failure.
- [High] Logic error due to false assumption of contiguous virtual IRQ numbers leads to unhandled INTx interrupts.
--

--- Patch [2]: [PATCH v3 2/3] PCI: rzg3s: Propagate platform_get_irq_byname() errors ---
commit 398dc4c2681ec621a5ea3ee51113abc78f26854a
Author: Lad Prabhakar <[email protected]>

PCI: rzg3s: Propagate platform_get_irq_byname() errors

platform_get_irq_byname() may return specific error codes such as
-EPROBE_DEFER, but rzg3s_pcie_init_irqdomain() replaces all failures
with -EINVAL. This commit propagates the original error code instead to
allow probe deferral.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index bd6471a7fb7a1..92371310d313b 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -975,7 +975,7 @@ static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
>  
>  		irq = platform_get_irq_byname(pdev, irq_name);
>  		if (irq < 0)
> -			return dev_err_probe(dev, -EINVAL,
> +			return dev_err_probe(dev, irq,

[Severity: High]
This is a pre-existing issue, but does this error path leave chained handlers
registered?

If rzg3s_pcie_init_irqdomain() successfully maps INTA but fails on a
subsequent INTx setup like an INTB probe deferral, returning directly leaves
the earlier chained handlers active. This same unhandled registration happens 
if irq_domain_create_linear() fails later, or if the teardown function 
omits unregistering the INTx chained handlers.

Since the device probe fails, the devres framework will free the host
structure. If the unhandled chained interrupt fires later, it will
dereference the freed host pointer, leading to a use-after-free panic:

rzg3s_pcie_intx_irq_handler()
    struct rzg3s_pcie_host *host = irq_desc_get_handler_data(desc);
    // UAF on host access

>  					     "Failed to parse and map INT%c IRQ\n",
>  					     'A' + i);
>  

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the INTx 
handling in this file, I noticed a potential issue in 
rzg3s_pcie_intx_irq_handler().

Is the calculation for the INTx index assuming contiguous virtual IRQ numbers?

drivers/pci/controller/pcie-rzg3s-host.c:rzg3s_pcie_intx_irq_handler() {
    ...
    u32 intx = irq - host->intx_irqs[0];
    ...
    generic_handle_domain_irq(host->intx_domain, intx);
}

If the OS allocates non-contiguous virqs (for example, due to concurrent IRQ
allocation or ID reuse by the IDR), this calculation will yield an invalid
out-of-bounds index or underflow.

Can this cause the generic handler lookup to fail? If so, leaving the INTx
interrupt unhandled might lead to an IRQ storm that permanently disables
the interrupt line and breaks downstream PCIe devices.

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