Fix: fd tracker: do not allow signal handlers to close lttng-ust FDs

Split the thread_fd_tracking state from the ust_fd_mutex_nest used to
track whether a signal handler is nested over a fd tracker lock.

lttng-ust listener threads need to invoke
lttng_ust_fd_tracker_register_thread() so the fd tracker can
distinguish them from application threads.

Otherwise, using ust_fd_mutex_nest to try to distinguish between
ust and application threads makes it possible for signal handlers
to appear as if they are ust listener threads, and thus attempt to
close UST file descriptors.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fixes: #1199

diff --git a/include/ust-fd.h b/include/ust-fd.h
index a0155251f4a55a2918e078880e3682dc92a47dc8..327a5f95086520d17576d61911c6c14885202d7a 100644 (file)
--- a/include/ust-fd.h
+++ b/include/ust-fd.h
@@ -31,6 +31,7 @@ int lttng_ust_add_fd_to_tracker(int fd);
 void lttng_ust_delete_fd_from_tracker(int fd);
 void lttng_ust_lock_fd_tracker(void);
 void lttng_ust_unlock_fd_tracker(void);
+void lttng_ust_fd_tracker_register_thread(void);
 
 int lttng_ust_safe_close_fd(int fd, int (*close_cb)(int));
 int lttng_ust_safe_fclose_stream(FILE *stream, int (*fclose_cb)(FILE *stream));

diff --git a/liblttng-ust-comm/lttng-ust-fd-tracker.c b/liblttng-ust-comm/lttng-ust-fd-tracker.c
index 9659f349e54ac85a3afad50b2cfc2055abcfacfc..42c3984cc2d3a390f941005f82a0315aa329c208 100644 (file)
--- a/liblttng-ust-comm/lttng-ust-fd-tracker.c
+++ b/liblttng-ust-comm/lttng-ust-fd-tracker.c
@@ -77,12 +77,20 @@ static int ust_safe_guard_saved_cancelstate;
 
 /*
  * Track whether we are within lttng-ust or application, for close
- * system call override by LD_PRELOAD library. This also tracks whether
- * we are invoking close() from a signal handler nested on an
- * application thread.
+ * system call override by LD_PRELOAD library. Threads registered
+ * as being lttng-ust listener threads need to perform fd tracker
+ * locking explicitly around their use of file descriptor manipulation
+ * functions.
+ */
+static DEFINE_URCU_TLS(int, thread_fd_tracking);
+
+/*
+ * Track whether we are invoking close() from a signal handler nested on
+ * an application thread.
  */
 static DEFINE_URCU_TLS(int, ust_fd_mutex_nest);
 
+
 /* fd_set used to book keep fd being used by lttng-ust. */
 static fd_set *lttng_fd_set;
 static int lttng_ust_max_fd;
@@ -94,6 +102,7 @@ static int init_done;
  */
 void lttng_ust_fixup_fd_tracker_tls(void)
 {
+       asm volatile ("" : : "m" (URCU_TLS(thread_fd_tracking)));
        asm volatile ("" : : "m" (URCU_TLS(ust_fd_mutex_nest)));
 }
 
@@ -348,7 +357,7 @@ int lttng_ust_safe_close_fd(int fd, int (*close_cb)(int fd))
         * If called from lttng-ust, we directly call close without
         * validating whether the FD is part of the tracked set.
         */
-       if (URCU_TLS(ust_fd_mutex_nest))
+       if (URCU_TLS(thread_fd_tracking))
                return close_cb(fd);
 
        lttng_ust_lock_fd_tracker();
@@ -384,7 +393,7 @@ int lttng_ust_safe_fclose_stream(FILE *stream, int (*fclose_cb)(FILE *stream))
         * If called from lttng-ust, we directly call fclose without
         * validating whether the FD is part of the tracked set.
         */
-       if (URCU_TLS(ust_fd_mutex_nest))
+       if (URCU_TLS(thread_fd_tracking))
                return fclose_cb(stream);
 
        fd = fileno(stream);
@@ -447,7 +456,7 @@ int lttng_ust_safe_closefrom_fd(int lowfd, int (*close_cb)(int fd))
         * If called from lttng-ust, we directly call close without
         * validating whether the FD is part of the tracked set.
         */
-       if (URCU_TLS(ust_fd_mutex_nest)) {
+       if (URCU_TLS(thread_fd_tracking)) {
                for (i = lowfd; i < lttng_ust_max_fd; i++) {
                        if (close_cb(i) < 0) {
                                switch (errno) {
@@ -492,3 +501,8 @@ int lttng_ust_safe_closefrom_fd(int lowfd, int (*close_cb)(int fd))
 end:
        return ret;
 }
+
+void lttng_ust_fd_tracker_register_thread(void)
+{
+       URCU_TLS(thread_fd_tracking) = 1;
+}

diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index fe33c5d7cfe7a1fc4f32607f3904c02c2f671d05..d3e01251bb107a4e3e98f39bff5a46c9a3de7274 100644 (file)
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -1453,6 +1453,7 @@ void *ust_listener_thread(void *arg)
        long timeout;
 
        lttng_ust_fixup_tls();
+       lttng_ust_fd_tracker_register_thread();
        /*
         * If available, add '-ust' to the end of this thread's
         * process name