From: Jérémie Galarneau Date: Tue, 23 May 2023 23:35:10 +0000 (-0400) Subject: Fix: sessiond: incorrect use of exclusions array leads to crash X-Git-Url: https://git.liburcu.org/?p=lttng-tools.git;a=commitdiff_plain;h=dd7ef1243236f524e57b25baa038973e793d5d72 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 , 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=, reply_payload=0x7fa9d3ffda88, ust_global=, channel_name=) at cmd.c:472 #4 cmd_list_events (domain=, session=, channel_name=, 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 --- diff --git a/include/lttng/event-internal.hpp b/include/lttng/event-internal.hpp index df5a9fd45..830b00a50 100644 --- 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 d6d894cfb..90ab4cdc0 100644 --- a/src/bin/lttng-sessiond/client.cpp +++ b/src/bin/lttng-sessiond/client.cpp @@ -30,6 +30,7 @@ #include "utils.hpp" #include +#include #include #include #include @@ -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 79cb6fed8..ef4383ff2 100644 --- 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 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 c0d128f53..47c0bcbf7 100644 --- 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 15a699e24..03ed2aa10 100644 --- 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(exclusion_list), filter_expression, bytecode_len, (ctx && bytecode_len) ? &ctx->bytecode->b : nullptr,