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 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 <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibd36f1e69da984c7f27b55ec68e5e3fe06d7ac91

diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
index a3d508a7018d5df82c0f0157ec7ba43fd1b5479a..c798234ae90cc0a48eb68fbb9c236122c514caf2 100644 (file)
--- 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 33098d869eb1e2aa99700910eb0b54890b2ed9eb..bd4611ef1664b90f5d63a98049234dca2d048b53 100644 (file)
--- 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)
 {