Re: [PATCH v7 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing synchronization in `flexcan_chip_interrupts_disable()` leads to a race with interrupt handlers, which can leave hardware interrupts enabled after `free_irq()`.
--

commit 199c16c8ec6245fdb40faa9ca7c9456354adfb8c
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()

This commit disables all registered IRQ lines around IMASK/CTRL register
writes in flexcan_chip_interrupts_enable to prevent handlers from firing
while registers are inconsistent on multi-IRQ platforms.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 2e9df2d14b341..4eda54d2169e7 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
[ ... ]
> @@ -1522,14 +1522,28 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
>  {
[ ... ]
>  	priv->write(upper_32_bits(reg_imask), &regs->imask2);
>  	priv->write(lower_32_bits(reg_imask), &regs->imask1);
>  	enable_irq(dev->irq);
> +	if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
> +		enable_irq(priv->irq_secondary_mb);
> +	if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
> +		enable_irq(priv->irq_boff);
> +		enable_irq(priv->irq_err);
> +	}
>  }
>  
>  static void flexcan_chip_interrupts_disable(const struct net_device *dev)

[Severity: High]
This is a pre-existing issue, but does flexcan_chip_interrupts_disable also
need this same fix pattern applied?

Currently, flexcan_chip_interrupts_disable modifies the ctrl register
without first masking the IRQ lines via disable_irq.

If flexcan_close calls flexcan_chip_interrupts_disable while the hardware
is still running, could a concurrent state-change interrupt on another CPU
invoke flexcan_do_state? That handler calls flexcan_error_irq_enable,
which might overwrite regs->ctrl and inadvertently re-enable hardware error
interrupts.

Since flexcan_close immediately proceeds to call free_irq while the
hardware might still be active due to this race, does this expose a time
window where an unhandled interrupt storm can trigger?

If an interrupt storm occurs without a registered handler, it could force
the kernel to permanently disable the IRQ line. On platforms where IRQs are
shared (requested by IRQF_SHARED in flexcan_open), this could break other
devices sharing the line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831143449.12828-1-ciprianmarian.costea@oss.nxp.com?part=2