From 6c1db447784d69988f661477ad9eea7c0fddfa82 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 13 Oct 2022 14:28:59 +0100 Subject: [PATCH] Fix: sessiond: abort called on undefined client command MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 19f912db8). 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 Signed-off-by: Jérémie Galarneau Change-Id: Ibd36f1e69da984c7f27b55ec68e5e3fe06d7ac91 --- src/bin/lttng-sessiond/client.cpp | 7 +++++++ src/common/sessiond-comm/sessiond-comm.hpp | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/src/bin/lttng-sessiond/client.cpp b/src/bin/lttng-sessiond/client.cpp index a52062e07..97a6d9b72 100644 --- 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 index 4bcc77bbb..9894e9ba9 100644 --- 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) { -- 2.34.1