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

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

Pre-existing issues:
- [High] Concurrent execution of multiple flexcan IRQ handlers causes lockless list corruption in the can_rx_offload queue.
--

--- Patch [3]: [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line ---
commit 9f143e05fb5957d463ce1d05d4126d168798b0d8
Author: Ciprian Marian Costea <[email protected]>

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

This commit updates flexcan to process split rx/tx masks based on the
mailbox IRQ line for platforms like S32G2. It introduces MB0 and MB1 masks
and passes them to flexcan_do_mb() to only process the relevant mailbox
range for each IRQ.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> --- 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: High]
This is a pre-existing issue, but does this concurrent execution risk
corrupting the lockless rx-offload queues?

Since the primary and secondary mailbox IRQs are separate lines (e.g., on
S32G2), they can be dispatched concurrently on different CPUs when frames
arrive simultaneously for both mailbox groups.

If both IRQs execute at the same time:

CPU1
flexcan_irq_mb()
   flexcan_do_mb()
      can_rx_offload_irq_offload_timestamp()
         __skb_queue_add_sort(&offload->skb_irq_queue, ...)

CPU2
flexcan_irq_mb()
   flexcan_do_mb()
      can_rx_offload_irq_offload_timestamp()
         __skb_queue_add_sort(&offload->skb_irq_queue, ...)

Or if CPU2 reaches the finish step while CPU1 is still adding:

CPU2
flexcan_irq_mb()
   can_rx_offload_irq_finish()
      skb_queue_splice_tail_init(&offload->skb_irq_queue, ...)

Since __skb_queue_add_sort() and skb_queue_splice_tail_init() operate
locklessly on offload->skb_irq_queue, could this lead to corrupted linked
list next/prev pointers or infinite loops during list traversal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3