Re: [PATCH v6 6/9] PCI: rzg3s-host: Move IRQ domain setup code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] irq: race in INTx chained handler setup before domain creation
--

commit 3d8eb4132e9198313898445e6472b136f82e2b3d
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

PCI: rzg3s-host: Move IRQ domain setup code

Subsequent patches add support for the event IRQ to handle link
up/down events. The event IRQ handler will use
rzg3s_pcie_set_max_link_speed(). In preparation for adding event IRQ
support, move the IRQ domain initialization code after
rzg3s_pcie_set_max_link_speed().

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 40d5ef3e347e4..3b62b1be5b2a1 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
[ ... ]
> @@ -1169,6 +1095,79 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
[ ... ]
> +static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
> +{
> +	struct device *dev = host->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	int i, ret;
> +
> +	for (i = 0; i < PCI_NUM_INTX; i++) {
> +		char irq_name[5] = {0};
> +		int irq;
> +
> +		scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
> +
> +		irq = platform_get_irq_byname(pdev, irq_name);
> +		if (irq < 0) {
> +			ret = irq;
> +			dev_err_probe(dev, ret,
> +				      "Failed to parse and map INT%c IRQ\n",
> +				      'A' + i);
> +			goto teardown_intx;
> +		}
> +
> +		host->intx_irqs[i] = irq;
> +		irq_set_chained_handler_and_data(irq,
> +						 rzg3s_pcie_intx_irq_handler,
> +						 host);
> +	}
> +
> +	host->intx_domain = irq_domain_create_linear(dev_fwnode(dev),
> +						     PCI_NUM_INTX,

[Severity: High]
Does registering the chained handler here open a race window before the domain
is created?

If the interrupt fires after irq_set_chained_handler_and_data() but before
irq_domain_create_linear(), rzg3s_pcie_intx_irq_handler() will execute with
host->intx_domain still being NULL.

This would result in a call to:
    generic_handle_domain_irq(NULL, intx);

Since the domain is NULL, __irq_resolve_mapping() will fall back to
irq_default_domain, which could incorrectly map the hardware IRQ (usually 0-3)
to an unrelated system interrupt.

Can this lead to unpredictable panics or memory corruption if an unintended
handler is executed?

> +						     &rzg3s_pcie_intx_domain_ops,
> +						     host);
> +	if (!host->intx_domain) {
> +		ret = -EINVAL;
> +		dev_err_probe(dev, ret,
> +			      "Failed to add irq domain for INTx IRQs\n");
> +		goto teardown_intx;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007053645.1391545-1-claudiu.beznea@kernel.org?part=6