From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Thu, 28 Jan 2021 19:39:37 +0000 (-0500)
Subject: Fix: sessiond: double free on duplicate removal of tracer source
X-Git-Tag: v2.13.0-rc1~368
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=2fb5e9b7971e2408dce182fcf7e87cd81dcb388b;p=lttng-tools.git

Fix: sessiond: double free on duplicate removal of tracer source

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 <jeremie.galarneau@efficios.com>
Change-Id: I7b5ebf90b868faded47a4e9675e01e1fb2b77a70
---

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. */