From 48a143c09cc97bf7a2ace811277e7d60b294b5f6 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 7 Oct 2019 15:41:10 -0400 Subject: [PATCH] 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 Fixes: #1199 --- include/ust-fd.h | 1 + liblttng-ust-comm/lttng-ust-fd-tracker.c | 26 ++++++++++++++++++------ liblttng-ust/lttng-ust-comm.c | 1 + 3 files changed, 22 insertions(+), 6 deletions(-) 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 47ba36e5..3cebb981 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 -- 2.34.1