From 5804876495751c1e7785be01af45a7203af13fa3 Mon Sep 17 00:00:00 2001 From: Michael Jeanson Date: Tue, 9 Mar 2021 12:38:06 -0500 Subject: [PATCH] fix: liblttng-ust-fd async-signal-safe close() "close(2)" is documented as async-signal-safe (see signal-safety(7)) and as such our override function should also be. This means we shouldn't do lazy initialization of the function pointer to the original libc close symbol in the override function. If we move the initialization of the function pointer in the library constructor we risk breaking other constructors that may run before ours and call close(). A compromise is to explicitly do the initialization in the constructor but keep a lazy init scheme if close() is called before it is executed. The dlsym call is now done only once, if it fails the wrappers will return -1 and set errno to ENOSYS. Change-Id: I05c66d2f5d51b2022c6803ca215340fb9c00f5a8 Signed-off-by: Michael Jeanson Signed-off-by: Mathieu Desnoyers --- src/lib/lttng-ust-fd/lttng-ust-fd.c | 118 ++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 26 deletions(-) diff --git a/src/lib/lttng-ust-fd/lttng-ust-fd.c b/src/lib/lttng-ust-fd/lttng-ust-fd.c index 56efb457..d2ebcfbc 100644 --- a/src/lib/lttng-ust-fd/lttng-ust-fd.c +++ b/src/lib/lttng-ust-fd/lttng-ust-fd.c @@ -10,76 +10,142 @@ #include #include #include +#include #include #include "common/macros.h" #include "common/ust-fd.h" -static int (*__lttng_ust_fd_plibc_close)(int fd); -static int (*__lttng_ust_fd_plibc_fclose)(FILE *stream); +#define LTTNG_UST_DLSYM_FAILED_PTR 0x1 -static -void _lttng_ust_fd_ctor(void) - __attribute__((constructor)); -static -void _lttng_ust_fd_ctor(void) -{ - lttng_ust_common_ctor(); -} +static int (*__lttng_ust_fd_plibc_close)(int fd) = NULL; +static int (*__lttng_ust_fd_plibc_fclose)(FILE *stream) = NULL; +/* + * Use dlsym to find the original libc close() symbol and store it in + * __lttng_ust_fd_plibc_close. + */ static -int _lttng_ust_fd_libc_close(int fd) +void *_lttng_ust_fd_init_plibc_close(void) { - if (!__lttng_ust_fd_plibc_close) { + if (__lttng_ust_fd_plibc_close == NULL) { __lttng_ust_fd_plibc_close = dlsym(RTLD_NEXT, "close"); - if (!__lttng_ust_fd_plibc_close) { + + if (__lttng_ust_fd_plibc_close == NULL) { + __lttng_ust_fd_plibc_close = (void *) LTTNG_UST_DLSYM_FAILED_PTR; fprintf(stderr, "%s\n", dlerror()); - return -1; } } - return lttng_ust_safe_close_fd(fd, __lttng_ust_fd_plibc_close); + + return __lttng_ust_fd_plibc_close; } +/* + * Use dlsym to find the original libc fclose() symbol and store it in + * __lttng_ust_fd_plibc_fclose. + */ static -int _lttng_ust_fd_libc_fclose(FILE *stream) +void *_lttng_ust_fd_init_plibc_fclose(void) { - if (!__lttng_ust_fd_plibc_fclose) { + if (__lttng_ust_fd_plibc_fclose == NULL) { __lttng_ust_fd_plibc_fclose = dlsym(RTLD_NEXT, "fclose"); - if (!__lttng_ust_fd_plibc_fclose) { + + if (__lttng_ust_fd_plibc_fclose == NULL) { + __lttng_ust_fd_plibc_fclose = (void *) LTTNG_UST_DLSYM_FAILED_PTR; fprintf(stderr, "%s\n", dlerror()); - return -1; } } - return lttng_ust_safe_fclose_stream(stream, - __lttng_ust_fd_plibc_fclose); + + return __lttng_ust_fd_plibc_fclose; } +static +void _lttng_ust_fd_ctor(void) + __attribute__((constructor)); +static +void _lttng_ust_fd_ctor(void) +{ + lttng_ust_common_ctor(); + + /* + * Initialize the function pointers to the original libc symbols in the + * constructor since close() has to stay async-signal-safe and as such, + * we can't call dlsym() in the override functions. + */ + (void) _lttng_ust_fd_init_plibc_close(); + (void) _lttng_ust_fd_init_plibc_fclose(); +} + +/* + * Override the libc close() symbol with our own, allowing applications to + * close arbitrary file descriptors. If the fd is owned by lttng-ust, return + * -1, errno=EBADF instead of closing it. + * + * If dlsym failed to find the original libc close() symbol, return -1, + * errno=ENOSYS. + * + * There is a short window before the library constructor has executed where + * this wrapper could call dlsym() and thus not be async-signal-safe. + */ int close(int fd) { - return _lttng_ust_fd_libc_close(fd); + /* + * We can't retry dlsym here since close is async-signal-safe. + */ + if (_lttng_ust_fd_init_plibc_close() == (void *) LTTNG_UST_DLSYM_FAILED_PTR) { + errno = ENOSYS; + return -1; + } + + return lttng_ust_safe_close_fd(fd, __lttng_ust_fd_plibc_close); } /* - * Note: fcloseall() is not an issue because it fcloses only the - * streams it knows about, which differs from the problems caused by - * gnulib close_stdout(), which does an explicit fclose(stdout). + * Override the libc fclose() symbol with our own, allowing applications to + * close arbitrary streams. If the fd is owned by lttng-ust, return -1, + * errno=EBADF instead of closing it. + * + * If dlsym failed to find the original libc close() symbol, return -1, + * errno=ENOSYS. + * + * There is a short window before the library constructor has executed where + * this wrapper could call dlsym() and thus not be async-signal-safe. + * + * Note: fcloseall() is not an issue because it closes only the streams it + * knows about, which differs from the problems caused by gnulib + * close_stdout(), which does an explicit fclose(stdout). */ int fclose(FILE *stream) { - return _lttng_ust_fd_libc_fclose(stream); + if (_lttng_ust_fd_init_plibc_fclose() == (void *) LTTNG_UST_DLSYM_FAILED_PTR) { + errno = ENOSYS; + return -1; + } + + return lttng_ust_safe_fclose_stream(stream, + __lttng_ust_fd_plibc_fclose); } #if defined(__sun__) || defined(__FreeBSD__) /* Solaris and FreeBSD. */ void closefrom(int lowfd) { + if (_lttng_ust_fd_init_plibc_close() == (void *) LTTNG_UST_DLSYM_FAILED_PTR) { + return; + } + (void) lttng_ust_safe_closefrom_fd(lowfd, __lttng_ust_fd_plibc_close); } #elif defined(__NetBSD__) || defined(__OpenBSD__) /* NetBSD and OpenBSD. */ int closefrom(int lowfd) { + if (_lttng_ust_fd_init_plibc_close() == (void *) LTTNG_UST_DLSYM_FAILED_PTR) { + errno = ENOSYS; + return -1; + } + return lttng_ust_safe_closefrom_fd(lowfd, __lttng_ust_fd_plibc_close); } #else -- 2.34.1