Re: CAN-XL frame to not CAN-XL enabled interface

[Vincent Mailhol <mailhol@kernel.org>]

On 31/07/2026 at 13:02, Marc Kleine-Budde wrote:
> Vincent,
> 
> does the current code check if you send a CAN XL frame to a CAN XL
> enabled interface?
> 
> On 31.07.2026 10:54:02, sashiko-bot@kernel.org wrote:
>> This is a pre-existing issue, but does this code properly handle CAN XL frames?
>>
>> If an ETH_P_CANXL frame is sent via AF_PACKET, can_dev_dropped_skb() currently
>> lacks a check to drop CAN XL frames for devices that don't support CAN XL.
>> When such a frame enters rkcanfd_start_xmit(), can_is_canfd_skb() returns
>> false, causing the driver to treat it as a Classic CAN frame.
> 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260731-master-v5-0-5b27029dee20@qq.com?part=1

The logic is for checking an ETH_P_CANXL frame is:

  - When sending a packet (even with PF_PACKET) the only check which
    is performed is to check that the skb len is not above
    net_device->mtu. So, if the device is not CAN XL capable, anything
    about CANFD_MTU (72 bytes) is dropped.

  - In the driver, can_dev_dropped_skb() calls
    can_dropped_invalid_skb() which then call can_is_canxl_skb() which
    validate that the skb length is within the range

      [CANXL_HDR_SIZE + CANXL_MIN_DLEN, skb->len > CANXL_MTU]

    that is to say 13 and 2060. But we already establish that the
    maximum length is 72 at this time.

So, yes, it seems that an ETH_P_CANXL frame with a length between 13
and 72 will pass the can_dev_dropped_skb() check. And because
CANXL_XLF must be set at this point, the driver sees a can_frame->len
of at least 128 bytes (and up to 255).

Until now, I was convinced in my mind that can_is_canxl_skb() rejected
frames with a length below CANXL_MIN_MTU which would have prevented
the issue. And I wrote the implementation under this false assumption
without double checking.


Here is a potential fix:

---8<---
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 6d0710d6f571..88dec0f91d71 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -152,6 +152,7 @@ static inline bool can_dev_in_xl_only_mode(struct can_priv *priv)
 /* drop skb if it does not contain a valid CAN frame for sending */
 static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *skb)
 {
+       const struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
        struct can_priv *priv = netdev_priv(dev);
        u32 silent_mode = priv->ctrlmode & (CAN_CTRLMODE_LISTENONLY |
                                            CAN_CTRLMODE_RESTRICTED);
@@ -167,6 +168,11 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
                goto invalid_skb;
        }
 
+       if (!(priv->ctrlmode & CAN_CTRLMODE_XL) && cxl->flags & CANXL_XLF) {
+               netdev_info_once(dev, "CAN XL is disabled, dropping skb\n");
+               goto invalid_skb;
+       }
+
        if (can_dev_in_xl_only_mode(priv) && !can_is_canxl_skb(skb)) {
                netdev_info_once(dev,
                                 "Error signaling is disabled, dropping skb\n");
---8<---

The cxl->flags & CANXL_XLF doesn't catch everything by itself, but
once we know that this flag is off, can_is_canxl_skb() will reject the
frame later on. So this should be the minimum fix
(can_dev_dropped_skb() is an inline function in the hot path, so any
trick is welcome here, I think).

Does this make sense?


Yours sincerely,
Vincent Mailhol