From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Guy Harris <gharris@sonic.net>
Cc: wireshark-dev@wireshark.org,
	linux-can@vger.kernel.org,
	Oliver Hartkopp <socketcan@hartkopp.net>
Subject: [PATCH] socketcan: make CAN CC/FD/XL frame handling less invasive
Date: Mon, 18 Mar 2024 11:49:33 +0100
Message-ID: <20240318104933.24194-1-socketcan@hartkopp.net>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="us-ascii"

The different CAN frame types are defined by Linux SLL_P types in the
sll_protocol field and the length of the frame.

LINUX_SLL_P_CANXL:
The frame length for CAN XL can be 12 + 1 to 12 + 2048 (13 .. 2060) byte.
Additionally the CANXL_XLF flag has to be set.

LINUX_SLL_P_CAN and LINUX_SLL_P_CANFD:
The frame length for CAN CC is 16 byte and for CAN FD it is 72 byte.
Additionally the CANXL_XLF flag is cleared in CAN CC/FD frames.

Reduce the modification and 'sanitizing' efforts only to the above
requirements to be robust against incompliant content which might be
provided via PACKET sockets but do not manipulate further content.

This helps to identify uninitialized data and to support new features
in formerly reserved structure elements without changing the libpcap.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 pcap-linux.c | 98 ++++++----------------------------------------------
 1 file changed, 10 insertions(+), 88 deletions(-)

diff --git a/pcap-linux.c b/pcap-linux.c
index c6516c19..0afd75eb 100644
--- a/pcap-linux.c
+++ b/pcap-linux.c
@@ -3960,103 +3960,17 @@ static int pcap_handle_packet_mmap(
 		 * type is DLT_CAN_SOCKETCAN.  Fix up the header
 		 * provided by the code below us to match what
 		 * DLT_CAN_SOCKETCAN is expected to provide.
 		 */
 		if (sll->sll_hatype == ARPHRD_CAN) {
-			pcap_can_socketcan_hdr *canhdr = (pcap_can_socketcan_hdr *)bp;
 			uint16_t protocol = ntohs(sll->sll_protocol);
 
-			/*
-			 * Check the protocol field from the sll header.
-			 * If it's one of the known CAN protocol types,
-			 * make sure the appropriate flags are set, so
-			 * that a program can tell what type of frame
-			 * it is.
-			 *
-			 * The two flags are:
-			 *
-			 *   CANFD_FDF, which is in the fd_flags field
-			 *   of the CAN classic/CAN FD header;
-			 *
-			 *   CANXL_XLF, which is in the flags field
-			 *   of the CAN XL header, which overlaps
-			 *   the payload_length field of the CAN
-			 *   classic/CAN FD header.
-			 */
-			switch (protocol) {
-
-			case LINUX_SLL_P_CAN:
-				/*
-				 * CAN classic.
-				 *
-				 * Zero out the fd_flags and reserved
-				 * fields, in case they're uninitialized
-				 * crap, and clear the CANXL_XLF bit in
-				 * the payload_length field.
-				 *
-				 * This means that the CANFD_FDF flag isn't
-				 * set in the fd_flags field, and that
-				 * the CANXL_XLF bit isn't set in the
-				 * payload_length field, so this frame
-				 * will appear to be a CAN classic frame.
-				 */
-				canhdr->payload_length &= ~CANXL_XLF;
-				canhdr->fd_flags = 0;
-				canhdr->reserved1 = 0;
-				canhdr->reserved2 = 0;
-				break;
-
-			case LINUX_SLL_P_CANFD:
-				/*
-				 * Set CANFD_FDF in the fd_flags field,
-				 * and clear the CANXL_XLF bit in the
-				 * payload_length field, so this frame
-				 * will appear to be a CAN FD frame.
-				 */
-				canhdr->payload_length &= ~CANXL_XLF;
-				canhdr->fd_flags |= CANFD_FDF;
-
-				/*
-				 * Zero out all the unknown bits in fd_flags
-				 * and clear the reserved fields, so that
-				 * a program reading this can assume that
-				 * CANFD_FDF is set because we set it, not
-				 * because some uninitialized crap was
-				 * provided in the fd_flags field.
-				 *
-				 * (At least some LINKTYPE_CAN_SOCKETCAN
-				 * files attached to Wireshark bugs had
-				 * uninitialized junk there, so it does
-				 * happen.)
-				 *
-				 * Update this if Linux adds more flag bits
-				 * to the fd_flags field or uses either of
-				 * the reserved fields for FD frames.
-				 */
-				canhdr->fd_flags &= (CANFD_FDF|CANFD_ESI|CANFD_BRS);
-				canhdr->reserved1 = 0;
-				canhdr->reserved2 = 0;
-				break;
-
-			case LINUX_SLL_P_CANXL:
-				/*
-				 * CAN XL frame.
-				 *
-				 * Make sure the CANXL_XLF bit is set in
-				 * the payload_length field, so that
-				 * this frame will appear to be a
-				 * CAN XL frame.
-				 */
-				canhdr->payload_length |= CANXL_XLF;
-				break;
-			}
-
 			/*
 			 * Put multi-byte header fields in a byte-order
 			 *-independent format.
 			 */
-			if (canhdr->payload_length & CANXL_XLF) {
+			if (protocol == LINUX_SLL_P_CANXL) {
 				/*
 				 * This is a CAN XL frame.
 				 *
 				 * DLT_CAN_SOCKETCAN is specified as having
 				 * the Priority ID/VCID field in big--
@@ -4085,10 +3999,13 @@ static int pcap_handle_packet_mmap(
 				 * frames were captured are likely
 				 * to be little-endian processors.
 				 */
 				pcap_can_socketcan_xl_hdr *canxl_hdr = (pcap_can_socketcan_xl_hdr *)bp;
 
+				/* ensure CAN XL flag for CAN XL fame detection */
+				canxl_hdr->flags |= CANXL_XLF;
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 				/*
 				 * We're capturing on a little-endian
 				 * machine, so we put the priority/VCID
 				 * field into big-endian byte order, and
@@ -4118,19 +4035,24 @@ static int pcap_handle_packet_mmap(
 #else
 #error "Unknown byte order"
 #endif
 			} else {
 				/*
-				 * CAN or CAN FD frame.
+				 * CAN CC or CAN FD frame.
 				 *
 				 * DLT_CAN_SOCKETCAN is specified as having
 				 * the CAN ID and flags in network byte
 				 * order, but capturing on a CAN device
 				 * provides it in host byte order.  Convert
 				 * it to network byte order.
 				 */
+				pcap_can_socketcan_hdr *canhdr = (pcap_can_socketcan_hdr *)bp;
+
 				canhdr->can_id = htonl(canhdr->can_id);
+
+				/* make sure the CAN XL detection is not affected */
+				canhdr->payload_length &= ~CANXL_XLF;
 			}
 		}
 	}
 
 	if (handlep->filter_in_userland && handle->fcode.bf_insns) {
-- 
2.43.0