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 <mjeanson@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/src/lib/lttng-ust-fd/lttng-ust-fd.c b/src/lib/lttng-ust-fd/lttng-ust-fd.c
index 56efb457269c2f7e9f85478a65b2daab61b6d7ff..d2ebcfbc38be862b6effffdc490f7a7d0ce968cb 100644 (file)
--- 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 <sys/types.h>
 #include <unistd.h>
 #include <dlfcn.h>
+#include <errno.h>
 
 #include <lttng/ust-common.h>
 
 #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