[lttng-dev] [PATCH lttng-ust] Fix: race between lttng-ust getenv() and application setenv()
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Fri Mar 10 23:30:18 UTC 2017
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 at efficios.com>
---
liblttng-ust-ctl/ustctl.c | 1 +
liblttng-ust/Makefile.am | 2 +
liblttng-ust/getenv.c | 98 ++++++++++++++++++++++++++++++++++++++
liblttng-ust/getenv.h | 29 ++++-------
liblttng-ust/lttng-clock.c | 2 +-
liblttng-ust/lttng-getcpu.c | 2 +-
liblttng-ust/lttng-ust-comm.c | 9 ++--
liblttng-ust/lttng-ust-statedump.c | 5 +-
snprintf/core.c | 5 ++
9 files changed, 126 insertions(+), 27 deletions(-)
create mode 100644 liblttng-ust/getenv.c
diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
index 2af7914..4067758 100644
--- a/liblttng-ust-ctl/ustctl.c
+++ b/liblttng-ust-ctl/ustctl.c
@@ -2205,6 +2205,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 7edac26..e79591a 100644
--- a/liblttng-ust/Makefile.am
+++ b/liblttng-ust/Makefile.am
@@ -70,6 +70,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
index 0000000..b4dc73a
--- /dev/null
+++ b/liblttng-ust/getenv.c
@@ -0,0 +1,98 @@
+/*
+ * Copyright (C) 2017 - Mathieu Desnoyers <mathieu.desnoyers at 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 = <tng_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 = <tng_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 05864fb..14d7050 100644
--- 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 b24ff37..877b5d6 100644
--- a/liblttng-ust/lttng-clock.c
+++ b/liblttng-ust/lttng-clock.c
@@ -102,7 +102,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 7b608ef..7cc9faa 100644
--- a/liblttng-ust/lttng-getcpu.c
+++ b/liblttng-ust/lttng-getcpu.c
@@ -47,7 +47,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 651d2aa..43728ff 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -366,11 +366,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");
}
/*
@@ -471,7 +471,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)
@@ -538,7 +538,7 @@ static
void get_blocking_retry_timeout(void)
{
const char *str_blocking_retry_timeout =
- lttng_secure_getenv("LTTNG_UST_BLOCKING_RETRY_TIMEOUT");
+ lttng_getenv("LTTNG_UST_BLOCKING_RETRY_TIMEOUT");
if (str_blocking_retry_timeout) {
long timeout = strtol(str_blocking_retry_timeout, NULL, 10);
@@ -1679,6 +1679,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_init_fd_tracker();
lttng_ust_clock_init();
diff --git a/liblttng-ust/lttng-ust-statedump.c b/liblttng-ust/lttng-ust-statedump.c
index 171cbae..efa8a55 100644
--- a/liblttng-ust/lttng-ust-statedump.c
+++ b/liblttng-ust/lttng-ust-statedump.c
@@ -34,6 +34,7 @@
#include "lttng-tracer-core.h"
#include "lttng-ust-statedump.h"
#include "jhash.h"
+#include "getenv.h"
#define TRACEPOINT_DEFINE
#include "ust_lib.h" /* Only define. */
@@ -548,7 +549,7 @@ void lttng_ust_dl_update(void *ip)
{
struct dl_iterate_data data;
- if (getenv("LTTNG_UST_WITHOUT_BADDR_STATEDUMP"))
+ if (lttng_getenv("LTTNG_UST_WITHOUT_BADDR_STATEDUMP"))
return;
/*
@@ -582,7 +583,7 @@ void lttng_ust_dl_update(void *ip)
static
int do_baddr_statedump(void *owner)
{
- if (getenv("LTTNG_UST_WITHOUT_BADDR_STATEDUMP"))
+ if (lttng_getenv("LTTNG_UST_WITHOUT_BADDR_STATEDUMP"))
return 0;
lttng_ust_dl_update(LTTNG_UST_CALLER_IP());
ust_dl_table_statedump(owner);
diff --git a/snprintf/core.c b/snprintf/core.c
index 966b588..c017b83 100644
--- 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;
--
2.1.4
More information about the lttng-dev
mailing list