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 <[email protected]>

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);

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) {

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;

src/common/event.cpp

@@ -664,8 +664,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)

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,