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