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

[Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>]

On 8/31/2026 9:01 PM, sashiko-bot@kernel.org wrote:
> 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?
> 

multi-IRQ concurrency findings in this series should be covered by the 
following patchset [1]

[1] 
https://lore.kernel.org/all/20260901114848.500591-1-ciprianmarian.costea@oss.nxp.com/T/#t

Regards,
Ciprian