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.cpp b/src/bin/lttng-sessiond/client.cpp
index a52062e07f877ddc08ccf84604a56c5f2ff1fd59..97a6d9b724c56afe9c24f9839444b19c5af8c8e7 100644 (file)
--- 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 4bcc77bbbb3035ab08e7d2313e42a8538165f995..9894e9ba9d6512628478e2b525126dd5bb8c6e25 100644 (file)
--- 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)
 {