[PATCH net-next V2 12/13] selftests: drv-net: psp: Fix responder parsing

Tariq Toukan <[email protected]>
Newsgroups org.kernel.vger.linux-rdma,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.netdev
Message-ID <[email protected]>
From: Cosmin Ratiu <[email protected]>

The psp_responder accumulates received data in a buffer and parses
received messages from it, but a message that's split in two across
command and argument (e.g. "psp conn" and "1") will permanently choke
the parser, because the stand-alone arg is never parsed after discarding
the command.

This is mostly a theoretical issue since a sent TCP segment from psp.py
of the form "psp conn 1" will arrive in one piece to psp_responder, but
the AI tools complain about the possibility that it might get split, so
fix it now before another command with an argument is added in the next
patch.

Signed-off-by: Cosmin Ratiu <[email protected]>
Reviewed-by: Dragos Tatulea <[email protected]>
Signed-off-by: Tariq Toukan <[email protected]>
---
 .../selftests/drivers/net/psp_responder.c     | 38 +++++++++----------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/psp_responder.c b/tools/testing/selftests/drivers/net/psp_responder.c
index a26e7628bbb1..985161eb482b 100644
--- a/tools/testing/selftests/drivers/net/psp_responder.c
+++ b/tools/testing/selftests/drivers/net/psp_responder.c
@@ -185,22 +185,22 @@ run_session(struct ynl_sock *ys, struct opts *opts,
 			}
 
 			off += n;
-			n = off;
 
 #define __consume(sz)						\
 		({						\
-			if (n == (sz)) {			\
-				off = 0;			\
-			} else {				\
-				off -= (sz);			\
-				memmove(buf, &buf[(sz)], off);	\
-			}					\
+			off -= (sz);				\
+			memmove(buf, &buf[(sz)], off);		\
 		})
 
-#define cmd(_name)							\
+/* Only match once the command and its _extra_sz byte payload are both
+ * buffered, otherwise a split read would consume the name and strand
+ * the payload, desynchronizing the parser for good.
+ */
+#define cmd(_name, _extra_sz)						\
 		({							\
 			ssize_t sz = sizeof(_name);			\
-			bool match = n >= sz &&	!memcmp(buf, _name, sz); \
+			bool match = off >= sz + (_extra_sz) &&		\
+				!memcmp(buf, _name, sz);		\
 									\
 			if (match) {					\
 				dbg("command: " _name "\n");		\
@@ -213,10 +213,10 @@ run_session(struct ynl_sock *ys, struct opts *opts,
 			do {
 				consumed = false;
 
-				if (cmd("read len"))
+				if (cmd("read len", 0))
 					send_str(comm_sock, data_read);
 
-				if (cmd("data echo")) {
+				if (cmd("data echo", 0)) {
 					if (data_sock >= 0)
 						send(data_sock, "echo", 5,
 						     MSG_WAITALL);
@@ -224,7 +224,7 @@ run_session(struct ynl_sock *ys, struct opts *opts,
 						fprintf(stderr, "WARN: echo but no data sock\n");
 					send_ack(comm_sock);
 				}
-				if (cmd("data close")) {
+				if (cmd("data close", 0)) {
 					if (data_sock >= 0) {
 						close(data_sock);
 						data_sock = -1;
@@ -233,26 +233,22 @@ run_session(struct ynl_sock *ys, struct opts *opts,
 						race_close = true;
 					}
 				}
-				if (cmd("conn psp")) {
+				if (cmd("conn psp", 2)) {
 					if (accept_cfg != ACCEPT_CFG_NONE)
 						fprintf(stderr, "WARN: old conn config still set!\n");
 					accept_cfg = ACCEPT_CFG_PSP;
 					send_ack(comm_sock);
 					/* next two bytes are versions */
-					if (off >= 2) {
-						memcpy(&psp_vers, buf, 2);
-						__consume(2);
-					} else {
-						fprintf(stderr, "WARN: short conn psp command!\n");
-					}
+					memcpy(&psp_vers, buf, 2);
+					__consume(2);
 				}
-				if (cmd("conn clr")) {
+				if (cmd("conn clr", 0)) {
 					if (accept_cfg != ACCEPT_CFG_NONE)
 						fprintf(stderr, "WARN: old conn config still set!\n");
 					accept_cfg = ACCEPT_CFG_CLEAR;
 					send_ack(comm_sock);
 				}
-				if (cmd("exit"))
+				if (cmd("exit", 0))
 					should_quit = true;
 #undef cmd
 
-- 
2.44.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.