Fix: race between lttng-ust getenv() and application setenv()

The LTTng-UST listener threads invoke getenv(), which can cause issues
if the application issues setenv() concurrently. This is a legitimate
use by the application because it may have a single thread and not be
aware that it runs with liblttng-ust.

Fix this by keeping our own environment variable table for the variables
we care about. Initialize this table within the lttng-ust library
constructor, when we don't race with the application.

As this thread shows:
https://sourceware.org/bugzilla/show_bug.cgi?id=5069#c10

getenv() does _not_ appear to be thread-safe if an application uses
setenv() or putenv().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
index 0165786c256b18203f49e9951132ead745ad638e..b5f337f06f8760ab60b07c0b636841ecc734faed 100644 (file)
--- a/liblttng-ust-ctl/ustctl.c
+++ b/liblttng-ust-ctl/ustctl.c
@@ -2187,6 +2187,7 @@ static __attribute__((constructor))
 void ustctl_init(void)
 {
        init_usterr();
+       lttng_ust_getenv_init();        /* Needs init_usterr() to be completed. */
        lttng_ust_clock_init();
        lttng_ring_buffer_metadata_client_init();
        lttng_ring_buffer_client_overwrite_init();

diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
index 8b542b4487fddd630f1e8992d01582c89d023eb0..9d32481060ba29744f2fcb7078c06eaf9ba30fcc 100644 (file)
--- a/liblttng-ust/Makefile.am
+++ b/liblttng-ust/Makefile.am
@@ -65,6 +65,8 @@ liblttng_ust_support_la_SOURCES = \
        lttng-tracer.h \
        lttng-tracer-core.h \
        ust-core.c \
+       getenv.h \
+       getenv.c \
        lttng-ust-dynamic-type.c \
        lttng-rb-clients.h \
        lttng-ring-buffer-client.h \

diff --git a/liblttng-ust/getenv.c b/liblttng-ust/getenv.c
new file mode 100644 (file)
index 0000000..b4dc73a
--- /dev/null
+++ b/liblttng-ust/getenv.c
@@ -0,0 +1,98 @@
+/*
+ * Copyright (C) 2017 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; only
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <usterr-signal-safe.h>
+#include <helper.h>
+#include "getenv.h"
+
+enum lttng_env_secure {
+       LTTNG_ENV_SECURE,
+       LTTNG_ENV_NOT_SECURE,
+};
+
+struct lttng_env {
+       const char *key;
+       enum lttng_env_secure secure;
+       char *value;
+};
+
+static struct lttng_env lttng_env[] = {
+       /*
+        * LTTNG_UST_DEBUG is used directly by snprintf, because it
+        * needs to be already set for ERR() used in
+        * lttng_ust_getenv_init().
+        */
+       { "LTTNG_UST_DEBUG", LTTNG_ENV_NOT_SECURE, NULL, },
+
+       /* Env. var. which can be used in setuid/setgid executables. */
+       { "LTTNG_UST_WITHOUT_BADDR_STATEDUMP", LTTNG_ENV_NOT_SECURE, NULL, },
+       { "LTTNG_UST_REGISTER_TIMEOUT", LTTNG_ENV_NOT_SECURE, NULL, },
+
+       /* Env. var. which are not fetched in setuid/setgid executables. */
+       { "LTTNG_UST_CLOCK_PLUGIN", LTTNG_ENV_SECURE, NULL, },
+       { "LTTNG_UST_GETCPU_PLUGIN", LTTNG_ENV_SECURE, NULL, },
+       { "LTTNG_UST_BLOCKING_RETRY_TIMEOUT", LTTNG_ENV_SECURE, NULL, },
+       { "HOME", LTTNG_ENV_SECURE, NULL, },
+       { "LTTNG_HOME", LTTNG_ENV_SECURE, NULL, },
+};
+
+static
+int lttng_is_setuid_setgid(void)
+{
+       return geteuid() != getuid() || getegid() != getgid();
+}
+
+char *lttng_getenv(const char *name)
+{
+       size_t i;
+       struct lttng_env *e;
+       bool found = false;
+
+       for (i = 0; i < LTTNG_ARRAY_SIZE(lttng_env); i++) {
+               e = &lttng_env[i];
+
+               if (strcmp(e->key, name) == 0) {
+                       found = true;
+                       break;
+               }
+       }
+       if (!found) {
+               return NULL;
+       }
+       return e->value;
+}
+
+void lttng_ust_getenv_init(void)
+{
+       size_t i;
+
+       for (i = 0; i < LTTNG_ARRAY_SIZE(lttng_env); i++) {
+               struct lttng_env *e = &lttng_env[i];
+
+               if (e->secure == LTTNG_ENV_SECURE && lttng_is_setuid_setgid()) {
+                       ERR("Getting environment variable '%s' from setuid/setgid binary refused for security reasons.",
+                               e->key);
+                       continue;
+               }
+               e->value = getenv(e->key);
+       }
+}

diff --git a/liblttng-ust/getenv.h b/liblttng-ust/getenv.h
index 05864fb8eb39b5c79ef3cdae2e31118f9edf7165..14d70506a388f9acff1ae600245d4d7f451fee13 100644 (file)
--- a/liblttng-ust/getenv.h
+++ b/liblttng-ust/getenv.h
@@ -19,26 +19,17 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#include <stdlib.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <usterr-signal-safe.h>
+/*
+ * Always add the lttng-ust environment variables to lttng_getenv()
+ * infrastructure rather than using getenv() directly from lttng-ust.
+ * This ensures that we don't trigger races between getenv() invoked by
+ * lttng-ust listener threads invoked concurrently with setenv() called
+ * by an otherwise single-threaded application thread. (the application
+ * is not aware that it runs with lttng-ust)
+ */
 
-static inline
-int lttng_is_setuid_setgid(void)
-{
-       return geteuid() != getuid() || getegid() != getgid();
-}
+char *lttng_getenv(const char *name);
 
-static inline
-char *lttng_secure_getenv(const char *name)
-{
-       if (lttng_is_setuid_setgid()) {
-               ERR("Getting environment variable '%s' from setuid/setgid binary refused for security reasons.",
-                       name);
-               return NULL;
-       }
-       return getenv(name);
-}
+void lttng_ust_getenv_init(void);
 
 #endif /* _COMPAT_GETENV_H */

diff --git a/liblttng-ust/lttng-clock.c b/liblttng-ust/lttng-clock.c
index 4299bcdec15767aca11f8ca2bfd5713cd6d8773d..d5aacf1d37674f4fe2723333dfab9357c6549f51 100644 (file)
--- a/liblttng-ust/lttng-clock.c
+++ b/liblttng-ust/lttng-clock.c
@@ -101,7 +101,7 @@ void lttng_ust_clock_init(void)
 
        if (clock_handle)
                return;
-       libname = lttng_secure_getenv("LTTNG_UST_CLOCK_PLUGIN");
+       libname = lttng_getenv("LTTNG_UST_CLOCK_PLUGIN");
        if (!libname)
                return;
        clock_handle = dlopen(libname, RTLD_NOW);

diff --git a/liblttng-ust/lttng-getcpu.c b/liblttng-ust/lttng-getcpu.c
index 751affac3cc29b747571ce786e2548be2d36f553..e4153a7c2b71f39750f2f375f90b015b53f96828 100644 (file)
--- a/liblttng-ust/lttng-getcpu.c
+++ b/liblttng-ust/lttng-getcpu.c
@@ -46,7 +46,7 @@ void lttng_ust_getcpu_init(void)
 
        if (getcpu_handle)
                return;
-       libname = lttng_secure_getenv("LTTNG_UST_GETCPU_PLUGIN");
+       libname = lttng_getenv("LTTNG_UST_GETCPU_PLUGIN");
        if (!libname)
                return;
        getcpu_handle = dlopen(libname, RTLD_NOW);

diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 38e66dc25926d64407d6fb48013849fc331c1198..61d5d2d87ad14df1cbd96fe813c6b782d680f9fe 100644 (file)
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -356,11 +356,11 @@ const char *get_lttng_home_dir(void)
 {
        const char *val;
 
-       val = (const char *) lttng_secure_getenv("LTTNG_HOME");
+       val = (const char *) lttng_getenv("LTTNG_HOME");
        if (val != NULL) {
                return val;
        }
-       return (const char *) lttng_secure_getenv("HOME");
+       return (const char *) lttng_getenv("HOME");
 }
 
 /*
@@ -460,7 +460,7 @@ long get_timeout(void)
        long constructor_delay_ms = LTTNG_UST_DEFAULT_CONSTRUCTOR_TIMEOUT_MS;
 
        if (!got_timeout_env) {
-               str_timeout = getenv("LTTNG_UST_REGISTER_TIMEOUT");
+               str_timeout = lttng_getenv("LTTNG_UST_REGISTER_TIMEOUT");
                got_timeout_env = 1;
        }
        if (str_timeout)
@@ -1597,6 +1597,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
         * sessiond before the init functions are completed).
         */
        init_usterr();
+       lttng_ust_getenv_init();        /* Needs init_usterr() to be completed. */
        init_tracepoint();
        lttng_ust_clock_init();
        lttng_ust_getcpu_init();

diff --git a/liblttng-ust/lttng-ust-statedump.c b/liblttng-ust/lttng-ust-statedump.c
index e1fbe0536d6cd3a4dadc44ef2afee4e0d41496fa..ea80817bd9807fd99cdfdabb8d12a2e58c4015c3 100644 (file)
--- a/liblttng-ust/lttng-ust-statedump.c
+++ b/liblttng-ust/lttng-ust-statedump.c
@@ -31,6 +31,7 @@
 #include <lttng/ust-elf.h>
 #include "lttng-tracer-core.h"
 #include "lttng-ust-statedump.h"
+#include "getenv.h"
 
 #define TRACEPOINT_DEFINE
 #define TRACEPOINT_CREATE_PROBES
@@ -312,7 +313,7 @@ int do_baddr_statedump(void *owner)
 {
        struct dl_iterate_data data;
 
-       if (getenv("LTTNG_UST_WITHOUT_BADDR_STATEDUMP"))
+       if (lttng_getenv("LTTNG_UST_WITHOUT_BADDR_STATEDUMP"))
                return 0;
 
        /*

diff --git a/snprintf/core.c b/snprintf/core.c
index 966b588721aaf439bfd6ecfa1b4e74a617486a4f..c017b832220b0230ed2bb9cdeb8eed51fe451286 100644 (file)
--- a/snprintf/core.c
+++ b/snprintf/core.c
@@ -27,6 +27,11 @@ void init_usterr(void)
        char *ust_debug;
 
        if (ust_loglevel == UST_LOGLEVEL_UNKNOWN) {
+               /*
+                * This getenv is not part of lttng_getenv() because it
+                * is required to print ERR() performed during getenv
+                * initialization.
+                */
                ust_debug = getenv("LTTNG_UST_DEBUG");
                if (ust_debug)
                        ust_loglevel = UST_LOGLEVEL_DEBUG;