[RFC PATCH v2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()

[RFC PATCH v2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()

[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 was set in can_send() and is now also set in
raw_check_txframe() to re-use the indroduced can_is_canfd_skb_set_fdf()
function. 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 | 25 +++++++++++++++++++++++--
 net/can/af_can.c        |  7 +------
 net/can/raw.c           |  2 +-
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 1abc25a8d144..38d036b43280 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -111,12 +111,33 @@ static inline bool can_is_can_skb(const struct sk_buff *skb)
 
 static inline bool can_is_canfd_skb(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 */
-	return (skb->len == CANFD_MTU && cfd->len <= CANFD_MAX_DLEN);
+	if (skb->len != CANFD_MTU || cfd->len > CANFD_MAX_DLEN)
+		return false;
+
+	return cfd->flags & CANFD_FDF;
+}
+
+static inline bool can_is_canfd_skb_set_fdf(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.
+	 * 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. So we can surely tag it as CAN FD.
+	 */
+	if (skb->len == CANFD_MTU && cfd->len <= CANFD_MAX_DLEN) {
+		/* set CAN FD flag for CAN FD frames by default */
+		cfd->flags |= CANFD_FDF;
+		return true;
+	}
+
+	return false;
 }
 
 static inline bool can_is_canxl_skb(const struct sk_buff *skb)
 {
 	const struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
diff --git a/net/can/af_can.c b/net/can/af_can.c
index b2387a46794a..0caf75a9e27f 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -207,17 +207,12 @@ 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)) {
-		struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
-
+	} else if (can_is_canfd_skb_set_fdf(skb)) {
 		skb->protocol = htons(ETH_P_CANFD);
-
-		/* set CAN FD flag for CAN FD frames by default */
-		cfd->flags |= CANFD_FDF;
 	} else {
 		goto inval_skb;
 	}
 
 	/* Make sure the CAN frame can pass the selected CAN netdevice. */
diff --git a/net/can/raw.c b/net/can/raw.c
index 76b867d21def..f48b1f3fd6e8 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_set_fdf(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