Skip to content

Commit

Permalink
DAOS-13194 gurt: environment APIs hook (#12220)
Browse files Browse the repository at this point in the history
In order to prevent known race to occur due to lack of
locking in Glibc environment APIs (getenv()/[uns]setenv()/
putenv()/clearenv()), they have been overloaded and
strengthened in Gurt with hooks now all using a common
lock/mutex.

Libgurt is the preferred place for this as it is the lowest
layer in DAOS, so it will be the earliest to be loaded and
will ensure the hook to be installed as early as possible
and could prevent usage of LD_PRELOAD.

This will address the main lack of multi-thread protection
in the Glibc APIs but do not handle all unsafe use-cases
(like the change/removal of an env var when its value address
has already been grabbed by a previous getenv(), ...).

Change-Id: I38cda09746ddb4e79f0297fee26c2a22e1cb881b
Signed-off-by: Bruno Faccini <[email protected]>
  • Loading branch information
bfaccini authored and mjmac committed Nov 7, 2023
1 parent d7bca97 commit 74cfd6e
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/gurt/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def scons():

denv = env.Clone()

denv.AppendUnique(LIBS=['pthread', 'yaml', 'm'])
denv.AppendUnique(LIBS=['pthread', 'yaml', 'm', 'dl'])
denv.require('uuid')

gurt_targets = denv.SharedObject(SRC)
Expand Down
122 changes: 122 additions & 0 deletions src/gurt/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@
#include <math.h>
#include <malloc.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <dlfcn.h>
#include <pthread.h>

#include <gurt/common.h>
#include <gurt/atomic.h>

/* state buffer for DAOS rand and srand calls, NOT thread safe */
static struct drand48_data randBuffer = {0};
Expand Down Expand Up @@ -1275,3 +1281,119 @@ d_vec_pointers_append(struct d_vec_pointers *pointers, void *pointer)
pointers->p_len++;
return 0;
}

/**
* Overloads to hook the unsafe getenv()/[un]setenv()/putenv()/clearenv()
* functions from glibc.
* Libgurt is the preferred place for this as it is the lowest layer in DAOS,
* so it will be the earliest to be loaded and will ensure the hook to be
* installed as early as possible and could prevent usage of LD_PRELOAD.
* The idea is to strengthen all the environment APIs by using a common lock.
*
* XXX this will address the main lack of multi-thread protection in the Glibc
* APIs but do not handle all unsafe use-cases (like the change/removal of an
* env var when its value address has already been grabbed by a previous
* getenv(), ...).
*/

static pthread_rwlock_t hook_env_lock = PTHREAD_RWLOCK_INITIALIZER;
static char *(* ATOMIC real_getenv)(const char *);
static int (* ATOMIC real_putenv)(char *);
static int (* ATOMIC real_setenv)(const char *, const char *, int);
static int (* ATOMIC real_unsetenv)(const char *);
static int (* ATOMIC real_clearenv)(void);

static void bind_libc_symbol(void **real_ptr_addr, const char *name)
{
void *real_temp;

/* XXX __atomic_*() built-ins are used to avoid the need to cast
* each of the ATOMIC pointers of functions, that seems to be
* required to make Intel compiler happy ...
*/
if (__atomic_load_n(real_ptr_addr, __ATOMIC_RELAXED) == NULL) {
/* libc should be already loaded ... */
real_temp = dlsym(RTLD_NEXT, name);
if (real_temp == NULL) {
/* try after loading libc now */
void *handle;

handle = dlopen("libc.so.6", RTLD_LAZY);
D_ASSERT(handle != NULL);
real_temp = dlsym(handle, name);
D_ASSERT(real_temp != NULL);
}
__atomic_store_n(real_ptr_addr, real_temp, __ATOMIC_RELAXED);
}
}

static pthread_once_t init_real_symbols_flag = PTHREAD_ONCE_INIT;

static void init_real_symbols(void)
{
bind_libc_symbol((void **)&real_getenv, "getenv");
bind_libc_symbol((void **)&real_putenv, "putenv");
bind_libc_symbol((void **)&real_setenv, "setenv");
bind_libc_symbol((void **)&real_unsetenv, "unsetenv");
bind_libc_symbol((void **)&real_clearenv, "clearenv");
}

char *getenv(const char *name)
{
char *p;

pthread_once(&init_real_symbols_flag, init_real_symbols);
D_RWLOCK_RDLOCK(&hook_env_lock);
p = real_getenv(name);
D_RWLOCK_UNLOCK(&hook_env_lock);

return p;
}

int putenv(char *name)
{
int rc;

pthread_once(&init_real_symbols_flag, init_real_symbols);
D_RWLOCK_WRLOCK(&hook_env_lock);
rc = real_putenv(name);
D_RWLOCK_UNLOCK(&hook_env_lock);

return rc;
}

int setenv(const char *name, const char *value, int overwrite)
{
int rc;

pthread_once(&init_real_symbols_flag, init_real_symbols);
D_RWLOCK_WRLOCK(&hook_env_lock);
rc = real_setenv(name, value, overwrite);
D_RWLOCK_UNLOCK(&hook_env_lock);

return rc;
}

int unsetenv(const char *name)
{
int rc;

pthread_once(&init_real_symbols_flag, init_real_symbols);
D_RWLOCK_WRLOCK(&hook_env_lock);
rc = real_unsetenv(name);
D_RWLOCK_UNLOCK(&hook_env_lock);

return rc;
}

int clearenv(void)
{
int rc;

pthread_once(&init_real_symbols_flag, init_real_symbols);
D_RWLOCK_WRLOCK(&hook_env_lock);
rc = real_clearenv();
D_RWLOCK_UNLOCK(&hook_env_lock);

return rc;
}
2 changes: 1 addition & 1 deletion src/gurt/tests/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def scons():

test_env = env.Clone()
test_env.require('mercury', 'uuid')
test_env.AppendUnique(LIBS=['pthread', 'cmocka', 'm'])
test_env.AppendUnique(LIBS=['pthread', 'cmocka', 'm', 'dl'])
test_env.AppendUnique(CXXFLAGS=['-std=c++0x'])
tests = []

Expand Down
2 changes: 1 addition & 1 deletion src/tests/ftest/cart/utest/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def scons():

test_env = env.Clone()
test_env.require('mercury', 'uuid', 'cmocka')
test_env.AppendUnique(LIBS=['pthread', 'm'])
test_env.AppendUnique(LIBS=['pthread', 'm', 'dl'])
test_env.AppendUnique(CXXFLAGS=['-std=c++0x'])
test_env.AppendUnique(LIBPATH=LIBPATH)
test_env.AppendUnique(RPATH_FULL=LIBPATH)
Expand Down

0 comments on commit 74cfd6e

Please sign in to comment.