Re: [RESEND 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:
- [Critical] Concurrent execution of split mailbox IRQ handlers corrupts the lockless `skb_irq_queue`, leading to list corruption and kernel panics.
--

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

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

This patch introduces FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK and
FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK to split the RX/TX masks between the two
mailbox IRQ lines on S32G2 platforms. It updates flexcan_do_mb() to accept
a mb_mask parameter so each IRQ handler only processes its corresponding
mailboxes.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 7dde2e623defe..8b70952f7f05d 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);

[Severity: Critical]
This is a pre-existing issue and was not introduced by this patch, but is
this safe from concurrent execution? If both primary and secondary MB IRQ
lines assert simultaneously on SMP systems, different CPUs could execute
flexcan_irq_mb() concurrently.

Both handlers would call flexcan_do_mb(), which eventually calls
can_rx_offload_irq_offload_timestamp(). That offload function modifies the
shared lockless queue without synchronization:

can_rx_offload_irq_offload_timestamp() {
    ...
    __skb_queue_add_sort(&offload->skb_irq_queue, skb,
                         can_rx_offload_compare);
    ...
}

Could this concurrent lockless mutation of the doubly-linked list result
in list corruption or kernel panics when traffic arrives simultaneously for
mailboxes in both the mb-0 and mb-1 ranges?

>  
>  	if (handled)
>  		can_rx_offload_irq_finish(&priv->offload);

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