Re: [PATCH v7 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The split IRQ handlers fail to resolve multi-IRQ concurrency for device state processing, leading to lockless list corruption.
--

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

can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms

This commit splits the monolithic flexcan_irq handler into dedicated 
functions for mailbox, device state, and bus error processing. The split
handlers are then registered to specific multi-IRQ lines depending on the 
platform quirks.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f5d22c61503fa..2e9df2d14b341 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
[ ... ]
> +/* Process device state change events */
> +static irqreturn_t flexcan_do_state(struct net_device *dev)
> +{
[ ... ]
> +	/* state change interrupt or broken error state quirk fix is enabled */
> +	if ((reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> +	    (priv->devtype_data.quirks &
> +	    (FLEXCAN_QUIRK_BROKEN_WERR_STATE | FLEXCAN_QUIRK_BROKEN_PERR_STATE)))
> +		flexcan_irq_state(dev, reg_esr);

[Severity: High]
This is a pre-existing issue, but does calling flexcan_irq_state()
concurrently corrupt the lockless SKB queue?

If the boff and err handlers execute flexcan_do_state() concurrently,
they will both allocate error SKBs and insert them into
offload->skb_irq_queue using the lockless __skb_queue_add_sort().

Can this concurrent insertion corrupt the list pointers (skb->next and
skb->prev), potentially causing infinite loops or use-after-free
conditions?

[ ... ]
> +/* Bus error IRQ handler for multi-IRQ platforms */
> +static irqreturn_t flexcan_irq_berr(int irq, void *dev_id)
> +{
> +	struct net_device *dev = dev_id;
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	irqreturn_t handled;
> +
> +	handled = flexcan_do_berr(dev);
> +
> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_BROKEN_PERR_STATE)
> +		handled |= flexcan_do_state(dev);

[Severity: High]
This isn't a bug introduced by this patch, but could this multi-IRQ
setup lead to concurrent device state mutations?

On platforms like s32g2 and mcf5441x with FLEXCAN_QUIRK_NR_IRQ_3 and
FLEXCAN_QUIRK_BROKEN_PERR_STATE, a hardware bus error or state change
can trigger both the boff and berr interrupts on distinct IRQ lines.

Since flexcan_irq_berr() and flexcan_irq_boff() execute on different
CPUs, couldn't they both call flexcan_do_state(dev) at the same time?

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