From c668ba47b997cb8bc75771f4f06bb61161d25f31 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 --- liblttng-ust-fd/lttng-ust-fd.c | 115 +++++++++++++++++++++++++++------ 1 file changed, 95 insertions(+), 20 deletions(-) diff --git a/liblttng-ust-fd/lttng-ust-fd.c b/liblttng-ust-fd/lttng-ust-fd.c index 4818e02e..513e757a 100644 --- a/liblttng-ust-fd/lttng-ust-fd.c +++ b/liblttng-ust-fd/lttng-ust-fd.c @@ -24,66 +24,141 @@ #include #include #include +#include #include #include "usterr-signal-safe.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 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) { - (void) lttng_ust_safe_closefrom(lowfd, __lttng_ust_fd_plibc_close); + 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) { - return lttng_ust_safe_closefrom(lowfd, __lttng_ust_fd_plibc_close); + 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 /* As far as we know, this OS does not implement closefrom. */ -- 2.34.1