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 47ba36e5c9f1d9f1871c267469049708a7dd1e1e..3cebb9811348545bb5c9a9d59556ab60362f5690 100644 (file)
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -1457,6 +1457,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