Fix: sessiond: abort called on undefined client command

Issue observed -------------- When running in verbose mode, the session daemon calls abort() when it receives an unknown client command: #1 0x00007f66ffd69958 in raise () from /usr/lib/libc.so.6 #2 0x00007f66ffd5353d in abort () from /usr/lib/libc.so.6 #3 0x000055a671a6f6bd in lttcomm_sessiond_command_str (cmd=1633771873) at ../../../src/common/sessiond-comm/sessiond-comm.hpp:199 #4 0x000055a671a73897 in process_client_msg (cmd_ctx=0x7f66f5ff6d10, sock=0x7f66f5ff6c34, sock_error=0x7f66f5ff6c38) at client.cpp:1006 #5 0x000055a671a777fc in thread_manage_clients (data=0x55a673956100) at client.cpp:2622 #6 0x000055a671a6d290 in launch_thread (data=0x55a673956170) at thread.cpp:68 Cause ----- process_client_msg() logs the client command on entry. While it previously logged the numerical value, it now provides the string-ified version of the command id (since 19f912d). The lttcomm_sessiond_command_str() function aborts when it encounters an unknown command id. This is reasonable (in so far that it is how we handle these situations, typically). However, the validity of the command must be checked beforehand as it comes from an external (untrusted) source. Solution -------- Add lttcomm_sessiond_command_is_valid and tombstone command IDs to lttcomm_sessiond_command to ensure only valid command ids are passed to lttcomm_sessiond_command_str when logging. Drawbacks --------- None Reported-by: Michael Jeanson <[email protected]> Signed-off-by: Jérémie Galarneau <[email protected]> Change-Id: Ibd36f1e69da984c7f27b55ec68e5e3fe06d7ac91
jgalar · Oct 13, 2022 · 6c1db44 · 6c1db44
1 parent 53ee940
commit 6c1db44
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 0 deletions.
diff --git a/src/bin/lttng-sessiond/client.cpp b/src/bin/lttng-sessiond/client.cpp
@@ -1003,6 +1003,13 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock,
 	bool need_domain;
 	bool need_consumerd;
 
+	if (!lttcomm_sessiond_command_is_valid((lttcomm_sessiond_command) cmd_ctx->lsm.cmd_type)) {
+		ERR("Unknown client command received: command id = %" PRIu32,
+				cmd_ctx->lsm.cmd_type);
+		ret = LTTNG_ERR_UND;
+		goto error;
+	}
+
 	DBG("Processing client command '%s\' (%d)",
 		lttcomm_sessiond_command_str((lttcomm_sessiond_command) cmd_ctx->lsm.cmd_type),
 		cmd_ctx->lsm.cmd_type);

diff --git a/src/common/sessiond-comm/sessiond-comm.hpp b/src/common/sessiond-comm/sessiond-comm.hpp
@@ -49,6 +49,7 @@
 #endif
 
 enum lttcomm_sessiond_command {
+	LTTCOMM_SESSIOND_COMMAND_MIN                    = -1,
 	/* Tracer command */
 	LTTNG_ADD_CONTEXT                               = 0,
 	/* LTTNG_CALIBRATE used to be here */
@@ -103,8 +104,15 @@ enum lttcomm_sessiond_command {
 	LTTNG_CLEAR_SESSION                             = 50,
 	LTTNG_LIST_TRIGGERS                             = 51,
 	LTTNG_EXECUTE_ERROR_QUERY                       = 52,
+	LTTCOMM_SESSIOND_COMMAND_MAX,
 };
 
+static inline
+bool lttcomm_sessiond_command_is_valid(enum lttcomm_sessiond_command cmd)
+{
+	return cmd > LTTCOMM_SESSIOND_COMMAND_MIN && cmd < LTTCOMM_SESSIOND_COMMAND_MAX;
+}
+
 static inline
 const char *lttcomm_sessiond_command_str(enum lttcomm_sessiond_command cmd)
 {