From 2fb5e9b7971e2408dce182fcf7e87cd81dcb388b Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 28 Jan 2021 14:39:37 -0500 Subject: [PATCH] Fix: sessiond: double free on duplicate removal of tracer source MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit An unrelated bug (fixed in a separate commit) can cause an event source to be removed from the notification thread's monitored sources twice. The event source removal starts by searching for the source to remove based on the source pipe's read-end fd number and assumes that it will always be found. After iterating on the list, an assertion that `source_element` is not NULL is done in the assumption that NULL would mean that the source was not found. This is incorrect since, if the source is not found, `source_element` will simply point to the last element of the list, causing the assertion to succeed. Then, the last source in the list is torn down, but not removed from the list. This causes that event source to be free'd twice when it is actually removed later on. The assumption that an event source can always be found does not hold for the moment. For instance, when an application can exit, closing its end of the notification pipe, the notification thread could wake-up before the application management thread. In that case, the notification thread will react to the event by removing the application's source from its monitored sources. Then, when the application management thread wakes up, it will ask the notification thread to (again) remove the event source, which will fail as it will not be found. Signed-off-by: Jérémie Galarneau Change-Id: I7b5ebf90b868faded47a4e9675e01e1fb2b77a70 --- src/bin/lttng-sessiond/notification-thread-events.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c index c9a142413..212746faa 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.c +++ b/src/bin/lttng-sessiond/notification-thread-events.c @@ -1955,6 +1955,7 @@ int handle_notification_thread_command_remove_tracer_event_source( enum lttng_error_code *_cmd_result) { int ret = 0; + bool found = false; enum lttng_error_code cmd_result = LTTNG_OK; struct notification_event_tracer_event_source_element *source_element = NULL, *tmp; @@ -1968,11 +1969,19 @@ int handle_notification_thread_command_remove_tracer_event_source( tracer_event_source_fd, lttng_domain_type_str(source_element->domain)); cds_list_del(&source_element->node); + found = true; break; } - /* It should always be found. */ - assert(source_element); + if (!found) { + /* + * This is temporarily allowed since the poll activity set is + * not properly cleaned-up for the moment. This is adressed in + * an upcoming fix. + */ + source_element = NULL; + goto end; + } if (!source_element->is_fd_in_poll_set) { /* Skip the poll set removal. */ -- 2.34.1