From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Mon, 7 Oct 2019 19:41:10 +0000 (-0400)
Subject: Fix: fd tracker: do not allow signal handlers to close lttng-ust FDs
X-Git-Tag: v2.11.0-rc5~3
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=01be959de2f1879fcd54a47a74ff443e2ef12f90;p=lttng-ust.git

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 a0155251..327a5f95 100644
--- 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 9659f349..42c3984c 100644
--- 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 4d851978..f91f8fd9 100644
--- 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