This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Making getenv more multi-threading--robust?
- From: Stephan Bergmann <sbergman at redhat dot com>
- To: libc-alpha at sourceware dot org
- Date: Thu, 13 Sep 2012 12:56:47 +0200
- Subject: Making getenv more multi-threading--robust?
[I had initially erroneously sent this to libc-help instead.]
I stumbled over this when running the LibreOffice test suite and getting
sporadic crashes in getenv(3). It turns out that LibreOffice contains
many places that call getenv (mostly to check whether to enable one
tweak or another) at arbitrary times and from arbitrary threads, and
apparently also contains the odd call to setenv etc. to modify the
environment.
Though getenv is generally known to not be thread-safe, glibc already
contains measures to make concurrent calls to setenv etc. thread-safe,
so I wonder whether something like the attached
0001-Prevent-races-when-getenv-walks-__environ-while-sete.patch would
make sense. While this would not solve my problems with
(cross-platform) LibreOffice, it would help prevent glibc getenv crashes
as demonstrated by the little program in the patch's commit message.
Stephan
>From 99ee3bea07a169a6bcd49b26d300c3d4a04b679c Mon Sep 17 00:00:00 2001
From: Stephan Bergmann <sbergman@redhat.com>
Date: Thu, 13 Sep 2012 11:52:42 +0200
Subject: [PATCH] Prevent races when getenv walks __environ while setenv
modifies it
... so that the below program does not crash in getenv:
#include <pthread.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
static void * thread(void * unused) {
for (;;) {
getenv("NOT_SET");
}
}
int main() {
pthread_t t;
int n;
char b[sizeof "KEY9999"];
if (pthread_create(&t, NULL, &thread, NULL) != 0) {
abort();
}
for (n = 0; n != 10000; ++n) {
sprintf(b, "KEY%d", n);
if (setenv(b, "", 1) != 0) {
abort();
}
}
exit(EXIT_SUCCESS);
}
---
stdlib/envlock.h | 10 ++++++++++
stdlib/getenv.c | 19 ++++++++++++++++---
stdlib/setenv.c | 10 ++--------
3 files changed, 28 insertions(+), 11 deletions(-)
create mode 100644 stdlib/envlock.h
diff --git a/stdlib/envlock.h b/stdlib/envlock.h
new file mode 100644
index 0000000..8a1fd05
--- /dev/null
+++ b/stdlib/envlock.h
@@ -0,0 +1,10 @@
+#if _LIBC
+/* This lock protects against simultaneous modifications of `environ'. */
+# include <bits/libc-lock.h>
+__libc_lock_define (extern, __envlock)
+# define LOCK __libc_lock_lock (__envlock)
+# define UNLOCK __libc_lock_unlock (__envlock)
+#else
+# define LOCK
+# define UNLOCK
+#endif
diff --git a/stdlib/getenv.c b/stdlib/getenv.c
index 8398dab..5c14455 100644
--- a/stdlib/getenv.c
+++ b/stdlib/getenv.c
@@ -22,6 +22,7 @@
#include <string.h>
#include <unistd.h>
+#include "envlock.h"
/* Return the value of the environment variable NAME. This implementation
is tuned a bit in that it assumes no environment variable has an empty
@@ -37,8 +38,13 @@ getenv (name)
char **ep;
uint16_t name_start;
+ LOCK;
+
if (__environ == NULL || name[0] == '\0')
- return NULL;
+ {
+ UNLOCK;
+ return NULL;
+ }
if (name[1] == '\0')
{
@@ -63,7 +69,10 @@ getenv (name)
| (((unsigned char *) *ep)[1] << 8));
#endif
if (name_start == ep_start)
- return &(*ep)[2];
+ {
+ UNLOCK;
+ return &(*ep)[2];
+ }
}
}
else
@@ -88,10 +97,14 @@ getenv (name)
if (name_start == ep_start && !strncmp (*ep + 2, name, len)
&& (*ep)[len + 2] == '=')
- return &(*ep)[len + 3];
+ {
+ UNLOCK;
+ return &(*ep)[len + 3];
+ }
}
}
+ UNLOCK;
return NULL;
}
libc_hidden_def (getenv)
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index d1db356..e6901d3 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -44,15 +44,9 @@ extern char **environ;
# endif
#endif
+#include "envlock.h"
#if _LIBC
-/* This lock protects against simultaneous modifications of `environ'. */
-# include <bits/libc-lock.h>
-__libc_lock_define_initialized (static, envlock)
-# define LOCK __libc_lock_lock (envlock)
-# define UNLOCK __libc_lock_unlock (envlock)
-#else
-# define LOCK
-# define UNLOCK
+__libc_lock_define_initialized (, __envlock)
#endif
/* In the GNU C library we must keep the namespace clean. */
--
1.7.11.4