Fix: sessiond: double free on duplicate removal of tracer source
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 28 Jan 2021 19:39:37 +0000 (14:39 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 28 Jan 2021 23:02:28 +0000 (18:02 -0500)
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

src/bin/lttng-sessiond/notification-thread-events.c

index c9a142413fe5538bf4afdff1378f6c221f6d7097..212746faa1c7a50d67e3006f868908222a00d22c 100644 (file)
@@ -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. */
This page took 0.03651 seconds and 4 git commands to generate.