Re: [PATCH v7 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Concurrent execution of mailbox IRQ handlers causes `skb_irq_queue` list corruption.
--

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

can: flexcan: split rx/tx masks per mailbox IRQ line

On S32G2 with two mailbox IRQ lines, handlers process the full rx_mask/tx_mask
range. This patch introduces masks to split the range between the MB0 and MB1
IRQ lines.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 4eda54d2169e7..f28ff0e95b74f 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
[ ... ]
> @@ -1244,8 +1255,14 @@ static irqreturn_t flexcan_irq_mb(int irq, void *dev_id)
>  	struct net_device *dev = dev_id;
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	irqreturn_t handled;
> +	u64 mb_mask = ~0ULL;
> +
> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
> +		mb_mask = (irq == priv->irq_secondary_mb) ?
> +			  FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK :
> +			  FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK;
>  
> -	handled = flexcan_do_mb(dev);
> +	handled = flexcan_do_mb(dev, mb_mask);
>  
>  	if (handled)
>  		can_rx_offload_irq_finish(&priv->offload);

[Severity: Critical]
This is a pre-existing issue, but does concurrent execution of mailbox IRQ
handlers cause skb_irq_queue list corruption?

On multi-IRQ platforms like S32G2, CAN packets arriving concurrently on both
mailbox ranges (MB 0-7 and MB 8-63) will trigger both mb-0 and mb-1
interrupts. If these are routed to different CPUs, flexcan_irq_mb() runs
concurrently on both CPUs.

Both contexts then invoke can_rx_offload_irq_offload_timestamp() (inside
flexcan_do_mb()) and can_rx_offload_irq_finish(), which modify the shared
lockless list offload->skb_irq_queue without synchronization:

drivers/net/can/dev/rx-offload.c:can_rx_offload_irq_offload_timestamp() {
    ...
    __skb_queue_add_sort(&offload->skb_irq_queue, skb,
                         can_rx_offload_compare);
    ...
}

drivers/net/can/dev/rx-offload.c:can_rx_offload_irq_finish() {
    ...
    skb_queue_splice_tail_init(&offload->skb_irq_queue, &offload->skb_queue);
    ...
}

Can this lead to memory corruption (corrupted sk_buff_head list pointers)
and an immediate kernel panic in hardirq context when the list is accessed
or modified?

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