From 7a0b90089e190dcd7a891a8b63242c273124b6d7 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.c | 7 +++++++ src/common/sessiond-comm/sessiond-comm.h | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c index a3d508a70..c798234ae 100644 --- a/src/bin/lttng-sessiond/client.c +++ b/src/bin/lttng-sessiond/client.c @@ -1006,6 +1006,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((enum 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(cmd_ctx->lsm.cmd_type), cmd_ctx->lsm.cmd_type); diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h index 33098d869..bd4611ef1 100644 --- a/src/common/sessiond-comm/sessiond-comm.h +++ b/src/common/sessiond-comm/sessiond-comm.h @@ -54,6 +54,7 @@ #define LTTCOMM_ERR_INDEX(code) (code - LTTCOMM_CONSUMERD_COMMAND_SOCK_READY) enum lttcomm_sessiond_command { + LTTCOMM_SESSIOND_COMMAND_MIN = -1, /* Tracer command */ LTTNG_ADD_CONTEXT = 0, /* LTTNG_CALIBRATE used to be here */ @@ -108,8 +109,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