From d4333eb996dec315fb721074c2cf49adee04363b Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 6 Oct 2023 15:17:41 -0400 Subject: [PATCH] Fix: sessiond: cmd potentially used uninitialized MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit GCC reports the following warning: libtool: compile: g++ -std=gnu++11 -DHAVE_CONFIG_H -I../../../include -I../../../include -I../../../src -I../../../src -include config.h -I/home/mjeanson/opt/include -I/home/mjeanson/opt/include -DINSTALL_BIN_PATH=\"/home/mjeanson/opt/lib/lttng/libexec\" -DINSTALL_LIB_PATH=\"/home/mjeanson/opt/lib\" -fvisibility=hidden -fvisibility-inlines-hidden -fno-strict-aliasing -Wall -Wextra -Wmissing-declarations -Wnull-dereference -Wundef -Wredundant-decls -Wshadow -Wsuggest-attribute=format -Wwrite-strings -Wformat=2 -Wstrict-aliasing -Wmissing-noreturn -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Winit-self -Wno-incomplete-setjmp-declaration -Wno-gnu-folding-constant -Wno-sign-compare -Werror -pthread -g -O2 -MT notification-thread-events.lo -MD -MP -MF .deps/notification-thread-events.Tpo -c notification-thread-events.cpp -fPIC -DPIC -o .libs/notification-thread-events.o In file included from field.hpp:14, from ust-field-convert.hpp:11, from ust-app.hpp:14, from event-notifier-error-accounting.hpp:11, from notification-thread-events.cpp:12: ../../../src/vendor/optional.hpp: In function ‘int handle_notification_thread_command(notification_thread_handle*, notification_thread_state*)’: ../../../src/vendor/optional.hpp:877:17: error: ‘cmd’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 877 | return &data; | ^~~~ notification-thread-events.cpp:3181:45: note: ‘cmd’ was declared here 3181 | struct notification_thread_command *cmd; | When failing to pop a command, cmd can indeed be used uninitialized when jumping to the error label. Reported-by: Michael Jeanson Signed-off-by: Jérémie Galarneau Change-Id: I5e63451ed4936638e9fbe05146f16c86ea2e42e2 --- .../notification-thread-events.cpp | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/bin/lttng-sessiond/notification-thread-events.cpp b/src/bin/lttng-sessiond/notification-thread-events.cpp index 70774a975..8065cd732 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.cpp +++ b/src/bin/lttng-sessiond/notification-thread-events.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -3150,27 +3151,27 @@ end: return 0; } -static int pop_cmd_queue(struct notification_thread_handle *handle, - struct notification_thread_command **cmd) +static notification_thread_command *pop_cmd_queue(notification_thread_handle *handle) { - int ret; - uint64_t counter; + lttng::pthread::lock_guard queue_lock(handle->cmd_queue.lock); - pthread_mutex_lock(&handle->cmd_queue.lock); - ret = lttng_read(handle->cmd_queue.event_fd, &counter, sizeof(counter)); - if (ret != sizeof(counter)) { - ret = -1; - goto error_unlock; + uint64_t counter; + const auto read_ret = lttng_read(handle->cmd_queue.event_fd, &counter, sizeof(counter)); + if (read_ret != sizeof(counter)) { + if (read_ret < 0) { + LTTNG_THROW_POSIX("Failed to read counter value from event_fd", errno); + } else { + LTTNG_THROW_ERROR(lttng::format( + "Failed to read counter value from event_fd because of a truncated read: ret={}, expected read size={}", + read_ret, + sizeof(counter))); + } } - *cmd = cds_list_first_entry( + auto command = cds_list_first_entry( &handle->cmd_queue.list, struct notification_thread_command, cmd_list_node); - cds_list_del(&((*cmd)->cmd_list_node)); - ret = 0; - -error_unlock: - pthread_mutex_unlock(&handle->cmd_queue.lock); - return ret; + cds_list_del(&((command)->cmd_list_node)); + return command; } /* Returns 0 on success, 1 on exit requested, negative value on error. */ @@ -3178,10 +3179,12 @@ int handle_notification_thread_command(struct notification_thread_handle *handle struct notification_thread_state *state) { int ret; - struct notification_thread_command *cmd; + struct notification_thread_command *cmd = nullptr; - ret = pop_cmd_queue(handle, &cmd); - if (ret) { + try { + cmd = pop_cmd_queue(handle); + } catch (const std::exception& ex) { + ERR("Failed to get next notification thread command: %s", ex.what()); goto error; } @@ -3313,11 +3316,13 @@ int handle_notification_thread_command(struct notification_thread_handle *handle } end: - if (cmd->is_async) { - delete cmd; - cmd = nullptr; - } else { - cmd->command_completed_waker->wake(); + if (cmd) { + if (cmd->is_async) { + delete cmd; + cmd = nullptr; + } else { + cmd->command_completed_waker->wake(); + } } return ret; -- 2.34.1