* [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()
@ 2025-09-09 16:37 Oliver Hartkopp
2025-09-09 16:37 ` [RFC PATCH v6 2/2] can: reject CAN FD content when disabled on CAN FD/XL interfaces Oliver Hartkopp
2025-09-10 4:13 ` [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Vincent Mailhol
0 siblings, 2 replies; 5+ messages in thread
From: Oliver Hartkopp @ 2025-09-09 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
The check in can_is_canfd_skb() is about a length check of skb->len and
the CAN FD data length. As a skb length of CANFD_MTU can potentially be
created with a CAN XL frame with a data length of 60, the length check of
the CAN FD data length is used to detect CAN XL frames via its CANXL_XLF
flag which exceeds valid CAN FD data length values.
To make sure the CANFD_FDF flag can be safely used as a marker for CAN FD
frame skbs the bit is set in can_send() which is used by all PF_CAN
protocols. In the RX path alloc_canfd_skb() sets the CANFD_FDF flag.
The enforced CANFD_FDF check in can_is_canfd_skb() clears up the potential
uncertainty when using the skb->len check with the CANFD_MTU.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can/skb.h | 19 +++++++++++++++++--
net/can/af_can.c | 2 +-
net/can/raw.c | 2 +-
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 1abc25a8d144..09ab4dc83199 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -107,18 +107,33 @@ static inline bool can_is_can_skb(const struct sk_buff *skb)
/* the CAN specific type of skb is identified by its data length */
return (skb->len == CAN_MTU && cf->len <= CAN_MAX_DLEN);
}
-static inline bool can_is_canfd_skb(const struct sk_buff *skb)
+static inline bool can_is_canfd_skb_mtu_len(const struct sk_buff *skb)
{
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
- /* the CAN specific type of skb is identified by its data length */
+ /* The CAN specific type of skb is identified by its data length.
+ * A CAN XL frame skb might have a skb->len of CANFD_MTU but the
+ * skb would have the CANXL_XLF bit set (0x80 = 128) in the
+ * cfd->len field position which would intentionally break the
+ * CAN FD length check here.
+ */
return (skb->len == CANFD_MTU && cfd->len <= CANFD_MAX_DLEN);
}
+static inline bool can_is_canfd_skb(const struct sk_buff *skb)
+{
+ struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
+
+ if (!can_is_canfd_skb_mtu_len(skb))
+ return false;
+
+ return cfd->flags & CANFD_FDF;
+}
+
static inline bool can_is_canxl_skb(const struct sk_buff *skb)
{
const struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
if (skb->len < CANXL_HDR_SIZE + CANXL_MIN_DLEN || skb->len > CANXL_MTU)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index b2387a46794a..7fd2ed510440 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -207,11 +207,11 @@ int can_send(struct sk_buff *skb, int loop)
if (can_is_canxl_skb(skb)) {
skb->protocol = htons(ETH_P_CANXL);
} else if (can_is_can_skb(skb)) {
skb->protocol = htons(ETH_P_CAN);
- } else if (can_is_canfd_skb(skb)) {
+ } else if (can_is_canfd_skb_mtu_len(skb)) {
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
skb->protocol = htons(ETH_P_CANFD);
/* set CAN FD flag for CAN FD frames by default */
diff --git a/net/can/raw.c b/net/can/raw.c
index 76b867d21def..e5e3952b0e09 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -886,11 +886,11 @@ static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
/* Classical CAN -> no checks for flags and device capabilities */
if (can_is_can_skb(skb))
return CAN_MTU;
/* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
- if (ro->fd_frames && can_is_canfd_skb(skb) &&
+ if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
(mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
return CANFD_MTU;
/* CAN XL -> needs to be enabled and a CAN XL device */
if (ro->xl_frames && can_is_canxl_skb(skb) &&
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH v6 2/2] can: reject CAN FD content when disabled on CAN FD/XL interfaces
2025-09-09 16:37 [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Oliver Hartkopp
@ 2025-09-09 16:37 ` Oliver Hartkopp
2025-09-10 5:00 ` Vincent Mailhol
2025-09-10 4:13 ` [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Vincent Mailhol
1 sibling, 1 reply; 5+ messages in thread
From: Oliver Hartkopp @ 2025-09-09 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
The CAN XL devices can be configured as CAN XL only with 'xl on fd off'.
Also CAN FD interfaces can be configured to only support CAN CC 'fd off'.
Correctly reject CAN FD frames when FD is disabled for the outgoing CAN
FD/XL interfaces.
Virtual CAN interfaces (vcan, vxcan) are not affected as they have no such
CAN CC/FD/XL content configuration option.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can/dev.h | 12 ++++++++++++
net/can/raw.c | 12 +++++++-----
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 9a92cbe5b2cb..9fa139cc793e 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -183,10 +183,22 @@ struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
void free_candev(struct net_device *dev);
/* a candev safe wrapper around netdev_priv */
struct can_priv *safe_candev_priv(struct net_device *dev);
+static inline bool can_dev_ctrlmode_fd_on(struct net_device *dev)
+{
+ struct can_priv *priv = safe_candev_priv(dev);
+
+ /* check ctrlmode on real CAN interfaces */
+ if (priv)
+ return (priv->ctrlmode & CAN_CTRLMODE_FD);
+
+ /* virtual CAN FD/XL interfaces always support CAN FD */
+ return true;
+}
+
int open_candev(struct net_device *dev);
void close_candev(struct net_device *dev);
int can_change_mtu(struct net_device *dev, int new_mtu);
int can_eth_ioctl_hwts(struct net_device *netdev, struct ifreq *ifr, int cmd);
int can_ethtool_op_get_ts_info_hwts(struct net_device *dev,
diff --git a/net/can/raw.c b/net/can/raw.c
index e5e3952b0e09..d9df62e15ba4 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -879,24 +879,26 @@ static void raw_put_canxl_vcid(struct raw_sock *ro, struct sk_buff *skb)
cxl->prio &= CANXL_PRIO_MASK;
cxl->prio |= ro->tx_vcid_shifted;
}
}
-static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
+static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
+ struct net_device *dev)
{
/* Classical CAN -> no checks for flags and device capabilities */
if (can_is_can_skb(skb))
return CAN_MTU;
- /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
+ /* CAN FD -> needs to be enabled in a CAN FD or CAN XL device */
if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
- (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
+ (dev->mtu == CANFD_MTU || can_is_canxl_dev_mtu(dev->mtu)) &&
+ can_dev_ctrlmode_fd_on(dev))
return CANFD_MTU;
/* CAN XL -> needs to be enabled and a CAN XL device */
if (ro->xl_frames && can_is_canxl_skb(skb) &&
- can_is_canxl_dev_mtu(mtu))
+ can_is_canxl_dev_mtu(dev->mtu))
return CANXL_MTU;
return 0;
}
@@ -948,11 +950,11 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
goto free_skb;
err = -EINVAL;
/* check for valid CAN (CC/FD/XL) frame content */
- txmtu = raw_check_txframe(ro, skb, dev->mtu);
+ txmtu = raw_check_txframe(ro, skb, dev);
if (!txmtu)
goto free_skb;
/* only CANXL: clear/forward/set VCID value */
if (txmtu == CANXL_MTU)
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()
2025-09-09 16:37 [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Oliver Hartkopp
2025-09-09 16:37 ` [RFC PATCH v6 2/2] can: reject CAN FD content when disabled on CAN FD/XL interfaces Oliver Hartkopp
@ 2025-09-10 4:13 ` Vincent Mailhol
2025-09-10 7:11 ` Oliver Hartkopp
1 sibling, 1 reply; 5+ messages in thread
From: Vincent Mailhol @ 2025-09-10 4:13 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
On Wed. 10 sept. 2025 at 01:37, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> The check in can_is_canfd_skb() is about a length check of skb->len and
> the CAN FD data length. As a skb length of CANFD_MTU can potentially be
> created with a CAN XL frame with a data length of 60, the length check of
> the CAN FD data length is used to detect CAN XL frames via its CANXL_XLF
> flag which exceeds valid CAN FD data length values.
>
> To make sure the CANFD_FDF flag can be safely used as a marker for CAN FD
> frame skbs the bit is set in can_send() which is used by all PF_CAN
> protocols. In the RX path alloc_canfd_skb() sets the CANFD_FDF flag.
>
> The enforced CANFD_FDF check in can_is_canfd_skb() clears up the potential
> uncertainty when using the skb->len check with the CANFD_MTU.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> include/linux/can/skb.h | 19 +++++++++++++++++--
> net/can/af_can.c | 2 +-
> net/can/raw.c | 2 +-
> 3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
> index 1abc25a8d144..09ab4dc83199 100644
> --- a/include/linux/can/skb.h
> +++ b/include/linux/can/skb.h
> @@ -107,18 +107,33 @@ static inline bool can_is_can_skb(const struct sk_buff *skb)
>
> /* the CAN specific type of skb is identified by its data length */
> return (skb->len == CAN_MTU && cf->len <= CAN_MAX_DLEN);
> }
>
> -static inline bool can_is_canfd_skb(const struct sk_buff *skb)
> +static inline bool can_is_canfd_skb_mtu_len(const struct sk_buff *skb)
> {
> struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
>
> - /* the CAN specific type of skb is identified by its data length */
> + /* The CAN specific type of skb is identified by its data length.
> + * A CAN XL frame skb might have a skb->len of CANFD_MTU but the
> + * skb would have the CANXL_XLF bit set (0x80 = 128) in the
> + * cfd->len field position which would intentionally break the
> + * CAN FD length check here.
> + */
> return (skb->len == CANFD_MTU && cfd->len <= CANFD_MAX_DLEN);
> }
>
> +static inline bool can_is_canfd_skb(const struct sk_buff *skb)
> +{
> + struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
> +
> + if (!can_is_canfd_skb_mtu_len(skb))
> + return false;
> +
> + return cfd->flags & CANFD_FDF;
> +}
How does this new can_is_canfd_skb() operate if a can frame is sent
through the PF_PACKET interface?
Wouldn't the frame bypass the can_send() and reach the driver's xmit()
function without the CANFD_FDF flag set?
> static inline bool can_is_canxl_skb(const struct sk_buff *skb)
> {
> const struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
>
> if (skb->len < CANXL_HDR_SIZE + CANXL_MIN_DLEN || skb->len > CANXL_MTU)
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index b2387a46794a..7fd2ed510440 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -207,11 +207,11 @@ int can_send(struct sk_buff *skb, int loop)
>
> if (can_is_canxl_skb(skb)) {
> skb->protocol = htons(ETH_P_CANXL);
> } else if (can_is_can_skb(skb)) {
> skb->protocol = htons(ETH_P_CAN);
> - } else if (can_is_canfd_skb(skb)) {
> + } else if (can_is_canfd_skb_mtu_len(skb)) {
> struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
>
> skb->protocol = htons(ETH_P_CANFD);
>
> /* set CAN FD flag for CAN FD frames by default */
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 76b867d21def..e5e3952b0e09 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -886,11 +886,11 @@ static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
> /* Classical CAN -> no checks for flags and device capabilities */
> if (can_is_can_skb(skb))
> return CAN_MTU;
>
> /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
> - if (ro->fd_frames && can_is_canfd_skb(skb) &&
> + if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
> (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
> return CANFD_MTU;
>
> /* CAN XL -> needs to be enabled and a CAN XL device */
> if (ro->xl_frames && can_is_canxl_skb(skb) &&
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v6 2/2] can: reject CAN FD content when disabled on CAN FD/XL interfaces
2025-09-09 16:37 ` [RFC PATCH v6 2/2] can: reject CAN FD content when disabled on CAN FD/XL interfaces Oliver Hartkopp
@ 2025-09-10 5:00 ` Vincent Mailhol
0 siblings, 0 replies; 5+ messages in thread
From: Vincent Mailhol @ 2025-09-10 5:00 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
On Wed. 10 Sep. 2025 at 01:37, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> The CAN XL devices can be configured as CAN XL only with 'xl on fd off'.
> Also CAN FD interfaces can be configured to only support CAN CC 'fd off'.
> Correctly reject CAN FD frames when FD is disabled for the outgoing CAN
> FD/XL interfaces.
>
> Virtual CAN interfaces (vcan, vxcan) are not affected as they have no such
> CAN CC/FD/XL content configuration option.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> include/linux/can/dev.h | 12 ++++++++++++
> net/can/raw.c | 12 +++++++-----
> 2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 9a92cbe5b2cb..9fa139cc793e 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -183,10 +183,22 @@ struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
> void free_candev(struct net_device *dev);
>
> /* a candev safe wrapper around netdev_priv */
> struct can_priv *safe_candev_priv(struct net_device *dev);
>
> +static inline bool can_dev_ctrlmode_fd_on(struct net_device *dev)
Nitpick: I would rather name this can_dev_fd_on() because this
function is meant to also work with virtual interfaces (seeing
ctrlmode in the name may fool the user into believing this is only for
"real" devices).
> +{
> + struct can_priv *priv = safe_candev_priv(dev);
> +
> + /* check ctrlmode on real CAN interfaces */
> + if (priv)
> + return (priv->ctrlmode & CAN_CTRLMODE_FD);
> +
> + /* virtual CAN FD/XL interfaces always support CAN FD */
> + return true;
Not always! It still requires the frame to fit in the MTU:
dev->mtu == CANFD_MTU
> +}
> +
> int open_candev(struct net_device *dev);
> void close_candev(struct net_device *dev);
> int can_change_mtu(struct net_device *dev, int new_mtu);
> int can_eth_ioctl_hwts(struct net_device *netdev, struct ifreq *ifr, int cmd);
> int can_ethtool_op_get_ts_info_hwts(struct net_device *dev,
> diff --git a/net/can/raw.c b/net/can/raw.c
> index e5e3952b0e09..d9df62e15ba4 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -879,24 +879,26 @@ static void raw_put_canxl_vcid(struct raw_sock *ro, struct sk_buff *skb)
> cxl->prio &= CANXL_PRIO_MASK;
> cxl->prio |= ro->tx_vcid_shifted;
> }
> }
>
> -static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
> +static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
> + struct net_device *dev)
> {
> /* Classical CAN -> no checks for flags and device capabilities */
> if (can_is_can_skb(skb))
> return CAN_MTU;
>
> - /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
> + /* CAN FD -> needs to be enabled in a CAN FD or CAN XL device */
> if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
> - (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
> + (dev->mtu == CANFD_MTU || can_is_canxl_dev_mtu(dev->mtu)) &&
> + can_dev_ctrlmode_fd_on(dev))
> return CANFD_MTU;
>
> /* CAN XL -> needs to be enabled and a CAN XL device */
> if (ro->xl_frames && can_is_canxl_skb(skb) &&
> - can_is_canxl_dev_mtu(mtu))
> + can_is_canxl_dev_mtu(dev->mtu))
> return CANXL_MTU;
>
> return 0;
> }
>
> @@ -948,11 +950,11 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
> goto free_skb;
>
> err = -EINVAL;
>
> /* check for valid CAN (CC/FD/XL) frame content */
> - txmtu = raw_check_txframe(ro, skb, dev->mtu);
> + txmtu = raw_check_txframe(ro, skb, dev);
> if (!txmtu)
> goto free_skb;
>
> /* only CANXL: clear/forward/set VCID value */
> if (txmtu == CANXL_MTU)
So this is going in the right direction. More comments will follow in
my answer to the comments in the v5 thread.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()
2025-09-10 4:13 ` [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Vincent Mailhol
@ 2025-09-10 7:11 ` Oliver Hartkopp
0 siblings, 0 replies; 5+ messages in thread
From: Oliver Hartkopp @ 2025-09-10 7:11 UTC (permalink / raw)
To: Vincent Mailhol; +Cc: linux-can
On 10.09.25 06:13, Vincent Mailhol wrote:
> On Wed. 10 sept. 2025 at 01:37, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>> The check in can_is_canfd_skb() is about a length check of skb->len and
>> the CAN FD data length. As a skb length of CANFD_MTU can potentially be
>> created with a CAN XL frame with a data length of 60, the length check of
>> the CAN FD data length is used to detect CAN XL frames via its CANXL_XLF
>> flag which exceeds valid CAN FD data length values.
>>
>> To make sure the CANFD_FDF flag can be safely used as a marker for CAN FD
>> frame skbs the bit is set in can_send() which is used by all PF_CAN
>> protocols. In the RX path alloc_canfd_skb() sets the CANFD_FDF flag.
>>
>> The enforced CANFD_FDF check in can_is_canfd_skb() clears up the potential
>> uncertainty when using the skb->len check with the CANFD_MTU.
>>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> include/linux/can/skb.h | 19 +++++++++++++++++--
>> net/can/af_can.c | 2 +-
>> net/can/raw.c | 2 +-
>> 3 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
>> index 1abc25a8d144..09ab4dc83199 100644
>> --- a/include/linux/can/skb.h
>> +++ b/include/linux/can/skb.h
>> @@ -107,18 +107,33 @@ static inline bool can_is_can_skb(const struct sk_buff *skb)
>>
>> /* the CAN specific type of skb is identified by its data length */
>> return (skb->len == CAN_MTU && cf->len <= CAN_MAX_DLEN);
>> }
>>
>> -static inline bool can_is_canfd_skb(const struct sk_buff *skb)
>> +static inline bool can_is_canfd_skb_mtu_len(const struct sk_buff *skb)
>> {
>> struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
>>
>> - /* the CAN specific type of skb is identified by its data length */
>> + /* The CAN specific type of skb is identified by its data length.
>> + * A CAN XL frame skb might have a skb->len of CANFD_MTU but the
>> + * skb would have the CANXL_XLF bit set (0x80 = 128) in the
>> + * cfd->len field position which would intentionally break the
>> + * CAN FD length check here.
>> + */
>> return (skb->len == CANFD_MTU && cfd->len <= CANFD_MAX_DLEN);
>> }
>>
>> +static inline bool can_is_canfd_skb(const struct sk_buff *skb)
>> +{
>> + struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
>> +
>> + if (!can_is_canfd_skb_mtu_len(skb))
>> + return false;
>> +
>> + return cfd->flags & CANFD_FDF;
>> +}
>
> How does this new can_is_canfd_skb() operate if a can frame is sent
> through the PF_PACKET interface?
>
> Wouldn't the frame bypass the can_send() and reach the driver's xmit()
> function without the CANFD_FDF flag set?
>
Yes, but the drivers are widely using can_is_canfd_skb().
But your remark let's me rethink this entire patch.
When I started to integrate CAN XL I moved the canxl_frame.flags to the
position of can(fd)_frame.len to intentionally break the data length
check, when CANXL_XLF (0x80) is set.
When CANXL_XLF is not set with a skb->len == CANFD_MTU we simply have a
CAN FD frame here. Period.
That the Linux kernel generates CAN_FD frames with CANFD_FDF set since
commit 061834624c872 was only intended to help the user space and to
ease the use of struct canfd_frame as dual use CC/FD data structure.
The differentiation of CAN CC and CAN FD was always done by the
structure length (MTU) for more than 12 years. So when you e.g. replay
an old capture file with WireShark that does not have CANFD_FDF bit set,
it might render the capture file unusable. Which is an UAPI break.
I tend to drop this entire patch and will concentrate on the patch that
early gives a feedback to the user of the CAN_RAW socket when it is not
allowed to send specific CAN frame types.
Btw. this does not make those checks unnecessary on the driver level as
the outgoing skb's might have been created with PF_PACKET.
Best regards,
Oliver
>> static inline bool can_is_canxl_skb(const struct sk_buff *skb)
>> {
>> const struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
>>
>> if (skb->len < CANXL_HDR_SIZE + CANXL_MIN_DLEN || skb->len > CANXL_MTU)
>> diff --git a/net/can/af_can.c b/net/can/af_can.c
>> index b2387a46794a..7fd2ed510440 100644
>> --- a/net/can/af_can.c
>> +++ b/net/can/af_can.c
>> @@ -207,11 +207,11 @@ int can_send(struct sk_buff *skb, int loop)
>>
>> if (can_is_canxl_skb(skb)) {
>> skb->protocol = htons(ETH_P_CANXL);
>> } else if (can_is_can_skb(skb)) {
>> skb->protocol = htons(ETH_P_CAN);
>> - } else if (can_is_canfd_skb(skb)) {
>> + } else if (can_is_canfd_skb_mtu_len(skb)) {
>> struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
>>
>> skb->protocol = htons(ETH_P_CANFD);
>>
>> /* set CAN FD flag for CAN FD frames by default */
>> diff --git a/net/can/raw.c b/net/can/raw.c
>> index 76b867d21def..e5e3952b0e09 100644
>> --- a/net/can/raw.c
>> +++ b/net/can/raw.c
>> @@ -886,11 +886,11 @@ static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
>> /* Classical CAN -> no checks for flags and device capabilities */
>> if (can_is_can_skb(skb))
>> return CAN_MTU;
>>
>> /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
>> - if (ro->fd_frames && can_is_canfd_skb(skb) &&
>> + if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
>> (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
>> return CANFD_MTU;
>>
>> /* CAN XL -> needs to be enabled and a CAN XL device */
>> if (ro->xl_frames && can_is_canxl_skb(skb) &&
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-10 7:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-09 16:37 [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Oliver Hartkopp
2025-09-09 16:37 ` [RFC PATCH v6 2/2] can: reject CAN FD content when disabled on CAN FD/XL interfaces Oliver Hartkopp
2025-09-10 5:00 ` Vincent Mailhol
2025-09-10 4:13 ` [RFC PATCH v6 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb() Vincent Mailhol
2025-09-10 7:11 ` Oliver Hartkopp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox