getenv not thread safe
Michael Eager
eager@mvista.com
Tue Feb 24 01:18:00 GMT 2004
On an SMP system, it's possible that getenv may be walking the
environment while setenv is reallocating it. This results in a SEGV
when getenv accesses memory which has already been free'd.
Here's a patch which fixes this by having getenv() acquire the same
lock that setenv uses before walking the environment. A test case
is also provided.
2004-02-22 Michael Eager <eager@mvista.com>
* sysdeps/generic/setenv.c (setenv) Make lock variable
global so it can be shared with getenv().
* sysdeps/generic/getenv.c (getenv) Get lock before
walking environment.
Index: sysdeps/generic/getenv.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/generic/getenv.c,v
retrieving revision 1.10
diff -u -r1.10 getenv.c
--- sysdeps/generic/getenv.c 3 Aug 2002 12:59:00 -0000 1.10
+++ sysdeps/generic/getenv.c 24 Feb 2004 00:53:49 -0000
@@ -27,6 +27,17 @@
#define __environ environ
#endif
+#if _LIBC
+/* This lock protects against simultaneous modifications of `environ'. */
+# include <bits/libc-lock.h>
+__libc_lock_define_initialized (, envlock)
+# define LOCK __libc_lock_lock (envlock)
+# define UNLOCK __libc_lock_unlock (envlock)
+#else
+# define LOCK
+# define UNLOCK
+#endif
+
/* Return the value of the environment variable NAME. This implementation
is tuned a bit in that it assumes no environment variable has an empty
name which of course should always be true. We have a special case for
@@ -58,6 +69,7 @@
#error "Funny byte order."
# endif
#endif
+ LOCK;
for (ep = __environ; *ep != NULL; ++ep)
{
#if _STRING_ARCH_unaligned
@@ -67,8 +79,12 @@
| (((unsigned char *) *ep)[1] << 8));
#endif
if (name_start == ep_start)
- return &(*ep)[2];
+ {
+ UNLOCK;
+ return &(*ep)[2];
+ }
}
+ UNLOCK;
}
else
{
@@ -81,6 +97,7 @@
len -= 2;
name += 2;
+ LOCK;
for (ep = __environ; *ep != NULL; ++ep)
{
#if _STRING_ARCH_unaligned
@@ -92,8 +109,12 @@
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;
Index: sysdeps/generic/setenv.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/generic/setenv.c,v
retrieving revision 1.30
diff -u -r1.30 setenv.c
--- sysdeps/generic/setenv.c 11 Feb 2004 06:32:32 -0000 1.30
+++ sysdeps/generic/setenv.c 24 Feb 2004 00:53:49 -0000
@@ -48,7 +48,7 @@
#if _LIBC
/* This lock protects against simultaneous modifications of `environ'. */
# include <bits/libc-lock.h>
-__libc_lock_define_initialized (static, envlock)
+__libc_lock_define_initialized (, envlock)
# define LOCK __libc_lock_lock (envlock)
# define UNLOCK __libc_lock_unlock (envlock)
#else
Test case. Gets SEGV in getenv on SMP system, runs until canceled
on UP system.
#include <stdlib.h>
#include <pthread.h>
static const char* names[] = {
"ENV_0", "ENV_1", "ENV_2", "ENV_3",
"ENV_4", "ENV_5", "ENV_6", "ENV_7"
};
void* thread_getenv(void* param)
{
char* env;
int i;
for(;;){
for(i = 0; i < sizeof(names) / sizeof(char*); i++){
env = getenv(names[i]);
}
}
return NULL;
}
void* thread_setenv(void* param)
{
char* var;
int i, size;
size = 2;
for(;;){
var = (char*)malloc(size);
if(var == NULL) break;
memset(var, '0', size - 1);
var[size - 1] = 0;
for(i = 0; i < sizeof(names) / sizeof(char*); i++){
setenv(names[i], var, 1);
}
free(var);
size++;
}
return NULL;
}
void* thread_malloc(void* param)
{
int size;
char* ptr;
size = 16;
for(;;){
ptr = (char*)malloc(size);
memset(ptr, '1', size);
free(ptr);
size++;
}
return NULL;
}
int main(void)
{
pthread_t t_get, t_set, t_mem;
pthread_create(&t_get, NULL, thread_getenv, NULL);
pthread_create(&t_set, NULL, thread_setenv, NULL);
pthread_create(&t_mem, NULL, thread_malloc, NULL);
for(;;) sleep(1);
return 0;
}
--
Michael Eager eager@mvista.com 408-328-8426
MontaVista Software, Inc. 1237 E. Arques Ave., Sunnyvale, CA 94085
More information about the Libc-alpha
mailing list