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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Failure to clear chained INTx IRQ handlers on error paths and during teardown leads to a use-after-free on probe failure.
- [High] System suspend path accesses AXI registers after disabling the PM domain, causing an asynchronous external abort.
- [High] INTx handler calculates hardware IRQ index assuming virtual IRQs are contiguous, which can lead to IRQ storms and lockups.
--

--- Patch [2]: [PATCH 2/3] PCI: rzg3s: Propagate platform_get_irq_byname() errors ---
commit 24fb1c19eb92c4744be36d3ec0690bbefc19cef4
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

PCI: rzg3s: Propagate platform_get_irq_byname() errors

This commit modifies rzg3s_pcie_init_irqdomain() to propagate the specific
error code returned by platform_get_irq_byname() instead of overriding it
with -EINVAL. This ensures probe deferral works correctly if the interrupt
provider is not yet available.

> 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,
>  					     "Failed to parse and map INT%c IRQ\n",
>  					     'A' + i);
>  

[Severity: High]
This is a pre-existing issue, but returning directly from this loop on
error without cleaning up chained INTx handlers leads to a use-after-free.

If platform_get_irq_byname() fails for i > 0 (for example, by returning
-EPROBE_DEFER), does the driver leave chained handlers registered for the
previous iterations?

When the probe fails, the host structure is freed by devm.

If the parent interrupt fires during the probe deferral waiting period,
could the kernel execute rzg3s_pcie_intx_irq_handler() with a dangling
pointer?

Similarly, should rzg3s_pcie_teardown_irqdomain() clear these handlers?

It currently removes the IRQ domain but completely omits cleaning up the
INTx chained handlers, which might leave them dangling on unbind.


[Severity: High]
This is a pre-existing issue, but does the system suspend path access AXI
registers after disabling the PM domain?

In drivers/pci/controller/pcie-rzg3s-host.c:rzg3s_pcie_suspend_noirq():

	ret = pm_runtime_put_sync(dev);
	if (ret)
		return ret;
	
	clk_disable_unprepare(port->refclk);
	
	/* SoC-specific de-initialization */
	ret = data->config_deinit(host);

This disables the power domain before executing SoC-specific
de-initialization.

For RZ/G3E, rzg3e_pcie_config_deinit() performs a writel_relaxed() to
host->axi + RZG3S_PCI_RESET.

Can accessing the AXI register space without clocks trigger a hardware fault?


[Severity: High]
This is a pre-existing issue, but does the INTx handler incorrectly assume
virtual IRQs are strictly contiguous?

In drivers/pci/controller/pcie-rzg3s-host.c:rzg3s_pcie_intx_irq_handler():

	unsigned int irq = irq_desc_get_irq(desc);
	u32 intx = irq - host->intx_irqs[0];

This calculates the hardware interrupt index assuming dynamically allocated
virtual IRQ numbers from platform_get_irq_byname() are contiguous.

Because this driver opts into PROBE_PREFER_ASYNCHRONOUS, concurrent IRQ
allocations by other drivers could mean virqs are not sequential.

If intx evaluates to an out-of-bounds index, generic_handle_domain_irq()
will fail to find the mapping.

Could this leave the level-triggered INTx line asserted but unserviced,
resulting in an IRQ storm?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715223019.3002237-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2