From 1663fc32b3c17f57faa5cf4eef7610589a063729 Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Tue, 13 Oct 2026 19:31:43 +0000
Subject: [PATCH] slcanpty: use the SLCAN message length when parsing commands

pty2can() detected complete SLCAN messages by looking for the '\r'
termination but did not use this message length for parsing. The
content was read at fixed positions and the start of the following
message was derived from the parsed content.

Too short messages (e.g. 'T1\r') read the DLC and data from stale
buffer content and sent a phantom CAN frame. Following commands got
lost after short messages, unknown commands or frames with timestamp.

Terminate the current message at its '\r' position, check that the
CAN ID fits into the message and always continue with the next message
behind the '\r'.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 slcanpty.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/slcanpty.c b/slcanpty.c
index bb26610..54a9de2 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -58,6 +58,7 @@ static int pty2can(int pty, int socket, struct can_filter *fi,
 	static char buf[200];
 	char replybuf[10]; /* for answers to received commands */
 	unsigned int ptr;
+	unsigned int msglen; /* length of the current SLCAN message w/o '\r' */
 	struct can_frame frame;
 	int ret, i;
 	static unsigned int rxoffset = 0; /* points to the end of an received incomplete SLCAN message */
@@ -102,6 +103,7 @@ rx_restart:
 		return 0;
 	}
 
+	msglen = tmp;
 	cmd = buf[0];
 	buf[nbytes] = 0;
 
@@ -112,10 +114,11 @@ rx_restart:
 			pr_debug("%c", buf[tmp]);
 	pr_debug("\n");
 
+	/* terminate the current SLCAN message to not read the following content */
+	buf[msglen] = 0;
+
 	/* check for filter configuration commands */
 	if (cmd == 'm' || cmd == 'M') {
-		buf[9] = 0; /* terminate filter string */
-		ptr = 9;
 #if 0
 		/* the filter is no SocketCAN filter :-( */
 
@@ -140,14 +143,12 @@ rx_restart:
 	/* check for timestamp on/off command */
 	if (cmd == 'Z') {
 		*tstamp = buf[1] & 0x01;
-		ptr = 2;
 		goto rx_out_ack;
 	}
 
 	/* check for 'O'pen command */
 	if (cmd == 'O') {
 		setsockopt(socket, SOL_CAN_RAW, CAN_RAW_FILTER, fi, sizeof(struct can_filter));
-		ptr = 1;
 		*is_open = 1;
 		goto rx_out_ack;
 	}
@@ -155,7 +156,6 @@ rx_restart:
 	/* check for 'C'lose command */
 	if (cmd == 'C') {
 		setsockopt(socket, SOL_CAN_RAW, CAN_RAW_FILTER, NULL, 0);
-		ptr = 1;
 		*is_open = 0;
 		goto rx_out_ack;
 	}
@@ -164,14 +164,12 @@ rx_restart:
 	if (cmd == 'V') {
 		sprintf(replybuf, "V1013\r");
 		tmp = strlen(replybuf);
-		ptr = 1;
 		goto rx_out;
 	}
 	/* check for 'v'ersion command */
 	if (cmd == 'v') {
 		sprintf(replybuf, "v1014\r");
 		tmp = strlen(replybuf);
-		ptr = 1;
 		goto rx_out;
 	}
 
@@ -179,7 +177,6 @@ rx_restart:
 	if (cmd == 'N') {
 		sprintf(replybuf, "N4242\r");
 		tmp = strlen(replybuf);
-		ptr = 1;
 		goto rx_out;
 	}
 
@@ -187,29 +184,23 @@ rx_restart:
 	if (cmd == 'F') {
 		sprintf(replybuf, "F00\r");
 		tmp = strlen(replybuf);
-		ptr = 1;
 		goto rx_out;
 	}
 
 	/* correctly answer unsupported commands */
 	if (cmd == 'U') {
-		ptr = 2;
 		goto rx_out_ack;
 	}
 	if (cmd == 'S') {
-		ptr = 2;
 		goto rx_out_ack;
 	}
 	if (cmd == 's') {
-		ptr = 5;
 		goto rx_out_ack;
 	}
 	if (cmd == 'P' || cmd == 'A') {
-		ptr = 1;
 		goto rx_out_nack;
 	}
 	if (cmd == 'X') {
-		ptr = 2;
 		if (buf[1] & 0x01)
 			goto rx_out_ack;
 		else
@@ -217,16 +208,18 @@ rx_restart:
 	}
 
 	/* catch unknown commands */
-	if ((cmd != 't') && (cmd != 'T') && (cmd != 'r') && (cmd != 'R')) {
-		ptr = nbytes - 1;
+	if ((cmd != 't') && (cmd != 'T') && (cmd != 'r') && (cmd != 'R'))
 		goto rx_out_nack;
-	}
 
 	if (cmd & 0x20) /* tiny chars 'r' 't' => SFF */
 		ptr = 4; /* dlc position tiiid */
 	else
 		ptr = 9; /* dlc position Tiiiiiiiid */
 
+	/* the CAN ID has to fit into the current SLCAN message */
+	if (ptr > msglen)
+		goto rx_out_nack;
+
 	memset(&frame.data, 0, 8); /* clear data[] */
 
 	if ((cmd | 0x20) == 'r' && buf[ptr] != '0') {
@@ -249,7 +242,6 @@ rx_restart:
 
 		buf[ptr] = frame.can_dlc; /* restore following byte */
 		frame.can_dlc = 0;
-		ptr--; /* we have no dlc component in the violation case */
 
 	} else {
 		if (!(buf[ptr] >= '0' && buf[ptr] < '9'))
@@ -277,9 +269,6 @@ rx_restart:
 				goto rx_out_nack;
 			frame.data[i] |= tmp;
 		}
-		/* point to last real data */
-		if (frame.can_dlc)
-			ptr--;
 	}
 
 	ret = write(socket, &frame, sizeof(frame));
@@ -303,8 +292,8 @@ rx_out:
 	}
 
 	/* check if there is another command in this buffer */
-	if (nbytes > ptr + 1) {
-		for (tmp = 0, ptr++; ptr + tmp < nbytes; tmp++)
+	if (nbytes > msglen + 1) {
+		for (tmp = 0, ptr = msglen + 1; ptr + tmp < nbytes; tmp++)
 			buf[tmp] = buf[ptr + tmp];
 		nbytes = tmp;
 		goto rx_restart;