Re: [PATCH RESEND v4 4/8] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent execution of separate IRQ handlers silently corrupts the lockless skb_irq_queue in the RX offload core.
--

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

dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support

Add NXP S32N79 SoC compatible string and interrupt properties.
On S32N79, FlexCAN IP is integrated with two interrupt lines:
one for the mailbox interrupts (0-127) and one for signaling
bus errors and device state changes.

> diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> index f81d56f7c12a5..d098a44c2b9c2 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> +++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml

[ ... ]

> @@ -173,11 +174,38 @@ allOf:
>              - const: mb-1
>        required:
>          - interrupt-names
> -    else:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nxp,s32n79-flexcan
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: Message Buffer interrupt for mailboxes 0-127
> +            - description: Bus Error and Device state change interrupt

[Severity: High]
This is a pre-existing issue, but can the concurrent execution of these separate
IRQ handlers silently corrupt the lockless skb_irq_queue in the RX offload core?

Since multi-IRQ platforms (like the newly added S32N79) map different FlexCAN
events to distinct hardware IRQ lines, the interrupt handlers can be dispatched
concurrently on different CPUs.

For example, CPU1 could be executing flexcan_irq_mb() while CPU2 is executing
flexcan_irq_boff():

CPU1:
drivers/net/can/flexcan/flexcan-core.c:flexcan_irq_mb() {
    ...
    handled = flexcan_do_mb(dev, mb_irq);
    ...
}

CPU2:
drivers/net/can/flexcan/flexcan-core.c:flexcan_irq_boff() {
    ...
    handled = flexcan_do_state(dev);
    ...
}

Both of these paths eventually call into the RX offload core to enqueue
packets:

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

Because __skb_queue_add_sort() and __skb_queue_tail() are lockless queue
operations, won't concurrent insertions from multiple CPUs corrupt the
doubly-linked list pointers and lead to a kernel panic on subsequent accesses?

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