Fix: sessiond: incorrect use of exclusions array leads to crash

Issue observed
--------------

When using the CLI to list the configuration of a session that has an
event rule which makes use of multiple exclusions, the session daemon
crashes with the following stack trace:

  (gdb) bt
  #0  0x00007fa9ed401445 in ?? () from /usr/lib/libc.so.6
  #1  0x0000560cd5fc5199 in lttng_strnlen (str=0x615f6f6c6c6568 <error: Cannot access memory at address 0x615f6f6c6c6568>, max=256) at ../../src/common/compat/string.h:19
  #2  0x0000560cd5fc6b39 in lttng_event_serialize (event=0x7fa9cc01d8b0, exclusion_count=2, exclusion_list=0x7fa9cc011794, filter_expression=0x0, bytecode_len=0, bytecode=0x0, payload=0x7fa9d3ffda88) at event.c:767
  #3  0x0000560cd5f380b5 in list_lttng_ust_global_events (nb_events=<synthetic pointer>, reply_payload=0x7fa9d3ffda88, ust_global=<optimized out>, channel_name=<optimized out>) at cmd.c:472
  #4  cmd_list_events (domain=<optimized out>, session=<optimized out>, channel_name=<optimized out>, reply_payload=0x7fa9d3ffda88) at cmd.c:3860
  #5  0x0000560cd5f6d76a in process_client_msg (cmd_ctx=0x7fa9d3ffa710, sock=0x7fa9d3ffa5b0, sock_error=0x7fa9d3ffa5b4) at client.c:1890
  #6  0x0000560cd5f6f876 in thread_manage_clients (data=0x560cd7879490) at client.c:2629
  #7  0x0000560cd5f65a54 in launch_thread (data=0x560cd7879500) at thread.c:66
  #8  0x00007fa9ed32d44b in ?? () from /usr/lib/libc.so.6
  #9  0x00007fa9ed3b0e40 in ?? () from /usr/lib/libc.so.6

Cause
-----

lttng_event_serialize expects a `char **` list of exclusion names, as
provided by the other callsite in liblttng-ctl. However, the callsite in
list_lttng_ust_global_events passes pointer to the exclusions as stored
in lttng_event_exclusion.

lttng_event_exclusion contains an array of fixed-length strings (with a
stride of 256 bytes) which isn't an expected layout for
lttng_event_serialize.

Solution
--------

A temporary array of pointers is constructed before invoking
lttng_event_serialize to construct a list of exclusions with the layout
that lttng_event_serialize expects.

The array itself is reused for all events, limiting the number of
allocations.

Note
----

None.

Change-Id: I266a1cc9e9f18e0476177a0047b1d8f468110575
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/include/lttng/event-internal.hpp b/include/lttng/event-internal.hpp
index df5a9fd45224ecff0b6d13035770187cb983f806..830b00a50aabefa7a5050df284a01b15ea1e7167 100644 (file)
--- a/include/lttng/event-internal.hpp
+++ b/include/lttng/event-internal.hpp
@@ -162,8 +162,8 @@ ssize_t lttng_event_create_from_payload(struct lttng_payload_view *view,
 
 int lttng_event_serialize(const struct lttng_event *event,
                          unsigned int exclusion_count,
-                         char **exclusion_list,
-                         char *filter_expression,
+                         const char **exclusion_list,
+                         const char *filter_expression,
                          size_t bytecode_len,
                          struct lttng_bytecode *bytecode,
                          struct lttng_payload *payload);

diff --git a/src/bin/lttng-sessiond/client.cpp b/src/bin/lttng-sessiond/client.cpp
index d6d894cfbeced044cc463a43be08e9271af29a34..90ab4cdc02a38aef1aff28f4bfc1b7953f8d1421 100644 (file)
--- a/src/bin/lttng-sessiond/client.cpp
+++ b/src/bin/lttng-sessiond/client.cpp
@@ -30,6 +30,7 @@
 #include "utils.hpp"
 
 #include <common/compat/getenv.hpp>
+#include <common/exception.hpp>
 #include <common/tracker.hpp>
 #include <common/unix.hpp>
 #include <common/utils.hpp>
@@ -2583,24 +2584,36 @@ static void *thread_manage_clients(void *data)
                 * informations for the client. The command context struct contains
                 * everything this function may needs.
                 */
-               ret = process_client_msg(&cmd_ctx, &sock, &sock_error);
-               rcu_thread_offline();
-               if (ret < 0) {
-                       if (sock >= 0) {
-                               ret = close(sock);
-                               if (ret) {
-                                       PERROR("close");
+               try {
+                       ret = process_client_msg(&cmd_ctx, &sock, &sock_error);
+                       rcu_thread_offline();
+                       if (ret < 0) {
+                               if (sock >= 0) {
+                                       ret = close(sock);
+                                       if (ret) {
+                                               PERROR("close");
+                                       }
                                }
+                               sock = -1;
+                               /*
+                                * TODO: Inform client somehow of the fatal error. At
+                                * this point, ret < 0 means that a zmalloc failed
+                                * (ENOMEM). Error detected but still accept
+                                * command, unless a socket error has been
+                                * detected.
+                                */
+                               continue;
                        }
-                       sock = -1;
-                       /*
-                        * TODO: Inform client somehow of the fatal error. At
-                        * this point, ret < 0 means that a zmalloc failed
-                        * (ENOMEM). Error detected but still accept
-                        * command, unless a socket error has been
-                        * detected.
-                        */
-                       continue;
+               } catch (const std::bad_alloc& ex) {
+                       WARN_FMT("Failed to allocate memory while handling client request: {}",
+                                ex.what());
+                       ret = LTTNG_ERR_NOMEM;
+               } catch (const lttng::ctl::error& ex) {
+                       WARN_FMT("Client request failed: {}", ex.what());
+                       ret = ex.code();
+               } catch (const std::exception& ex) {
+                       WARN_FMT("Client request failed: {}", ex.what());
+                       ret = LTTNG_ERR_UNK;
                }
 
                if (ret < LTTNG_OK || ret >= LTTNG_ERR_NR) {

diff --git a/src/bin/lttng-sessiond/cmd.cpp b/src/bin/lttng-sessiond/cmd.cpp
index 79cb6fed89398e00833f2ea27620090eba09e81e..ef4383ff25a67262c2f6d5157422a916d94a9147 100644 (file)
--- a/src/bin/lttng-sessiond/cmd.cpp
+++ b/src/bin/lttng-sessiond/cmd.cpp
@@ -484,18 +484,28 @@ static enum lttng_error_code list_lttng_ust_global_events(char *channel_name,
                        tmp_event->exclusion = 1;
                }
 
+               std::vector<const char *> exclusion_names;
+               if (uevent->exclusion) {
+                       for (int i = 0; i < uevent->exclusion->count; i++) {
+                               exclusion_names.emplace_back(
+                                       LTTNG_EVENT_EXCLUSION_NAME_AT(uevent->exclusion, i));
+                       }
+               }
+
                /*
                 * We do not care about the filter bytecode and the fd from the
                 * userspace_probe_location.
                 */
-               ret = lttng_event_serialize(tmp_event,
-                                           uevent->exclusion ? uevent->exclusion->count : 0,
-                                           uevent->exclusion ? (char **) uevent->exclusion->names :
-                                                               nullptr,
-                                           uevent->filter_expression,
-                                           0,
-                                           nullptr,
-                                           reply_payload);
+               ret = lttng_event_serialize(
+                       tmp_event,
+                       exclusion_names.size(),
+                       exclusion_names.size() ?
+                               exclusion_names.data() :
+                               nullptr,
+                       uevent->filter_expression,
+                       0,
+                       nullptr,
+                       reply_payload);
                lttng_event_destroy(tmp_event);
                if (ret) {
                        ret_code = LTTNG_ERR_FATAL;

diff --git a/src/common/event.cpp b/src/common/event.cpp
index c0d128f53edede855184f53b4a8d502b10d5ccdf..47c0bcbf7e39f8ad32a8cf6c615987dfc263ae6d 100644 (file)
--- a/src/common/event.cpp
+++ b/src/common/event.cpp
@@ -664,8 +664,8 @@ end:
 
 int lttng_event_serialize(const struct lttng_event *event,
                          unsigned int exclusion_count,
-                         char **exclusion_list,
-                         char *filter_expression,
+                         const char **exclusion_list,
+                         const char *filter_expression,
                          size_t bytecode_len,
                          struct lttng_bytecode *bytecode,
                          struct lttng_payload *payload)

diff --git a/src/lib/lttng-ctl/lttng-ctl.cpp b/src/lib/lttng-ctl/lttng-ctl.cpp
index 15a699e24f3fd63837edbc6b11dccfa3ac5b1a10..03ed2aa10a4459891018fe038b7a8cbdbaf50c75 100644 (file)
--- a/src/lib/lttng-ctl/lttng-ctl.cpp
+++ b/src/lib/lttng-ctl/lttng-ctl.cpp
@@ -1180,7 +1180,7 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle,
 serialize:
        ret = lttng_event_serialize(ev,
                                    exclusion_count,
-                                   exclusion_list,
+                                   const_cast<const char **>(exclusion_list),
                                    filter_expression,
                                    bytecode_len,
                                    (ctx && bytecode_len) ? &ctx->bytecode->b : nullptr,