The branch, master has been updated via fdbdb8a uwrap: Reflect changes of uid/gid in threads to main process. via cd254f6 tests: Extend test_sync_setgid in test_glibc_thread_support.c via afc62c9 uwrap: Small optimalization of uwrap_init(). via 9b042d7 tests: test_glibc_thread_support: Add bigger load. via 9a4fc57 tests: Added two new tests aimed to setgid() and getgid() functions. via 6440f8e tests: Small phtread_attr cleanup. via a8d0bc7 tests: Get rid of malloc calls in test_glibc_thread_support.c. via e88afed uwrap: Optimalization of uid_wrapper_enabled() function. via 0c80f0c uid_wrapper: Fix race condition - uwrap_init. via 401d92a uwrap: Fix race condition - glibc lookups. via 724409c uwrap: Add library constructor and move pthread_atfork inside. via 973a784 uwrap: Use UWRAP_LOCK/UNLOCK macros instead of pthread_mutex_lock/unlock calls. from ba53521 uwrap: Fix the handle loop for older gcc versions.
http://gitweb.samba.org/?p=uid_wrapper.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit fdbdb8ad1a6e745b5b4e7bf7f714ff11700a1227 Author: Robin Hack <hack.ro...@gmail.com> Date: Mon Sep 29 09:53:21 2014 +0200 uwrap: Reflect changes of uid/gid in threads to main process. When thread changes uid/gid this change must be reflected to main process. Syscalls changes only uid/gid of thread. Call of libc functions changes also uid/gid of main process. TESTS: Fix test_sync_setgid_syscall in test_glibc_thread_support.c When changes of gid (and uid of course) is reflected to main process test must check if call of setgid syscall changes gid of thread ONLY. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit cd254f6c787cc008b4e926c5c8e84fa531543e4a Author: Robin Hack <hack.ro...@gmail.com> Date: Sun Sep 28 21:19:13 2014 +0200 tests: Extend test_sync_setgid in test_glibc_thread_support.c Extend test to case when thread changes gid of main process. After this change main process start new thread which should have same gid set as a main process. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit afc62c9c321c4d2be8246f90fd21a80096def09e Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 16:25:58 2014 +0200 uwrap: Small optimalization of uwrap_init(). Don't call getenv("UID_WRAPPER") on start of uwrap_init(). Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 9b042d73d8f80dea94090c2793281618895aa4e3 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 16:19:09 2014 +0200 tests: test_glibc_thread_support: Add bigger load. This can help (and helped) with revealing race conditions. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 9a4fc5759d5f5af59024dbe48d262277b7e91ce9 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 16:17:18 2014 +0200 tests: Added two new tests aimed to setgid() and getgid() functions. New tests: * test_sync_setgid - test if call of setgid() in thread changes gid of main process * test_sync_setgid_syscall - test setgid() syscall Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 6440f8e7f368c06b56622c9b9b443041d932db88 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 15:25:09 2014 +0200 tests: Small phtread_attr cleanup. This change should make valgrind happy. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit a8d0bc7279a44141477f508dcde177bee57957a7 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 14:31:39 2014 +0200 tests: Get rid of malloc calls in test_glibc_thread_support.c. Thread structures are allocated on stack now. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit e88afed067f2a04d2afd3a24458315143319871c Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 14:00:27 2014 +0200 uwrap: Optimalization of uid_wrapper_enabled() function. Check only bool variable inside uwrap structure instead of calling whole uid_init(). In the best case only one mutex lock is need when check. NOTES: * This patch uses __atomic_load gcc builtin function. * uid_init() were moved outside uid_wrapper_enabled() function. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 0c80f0ce34ce9eda7495b4620187ebfdb95b46b3 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 13:26:17 2014 +0200 uid_wrapper: Fix race condition - uwrap_init. Patch moves uwrap_id_mutex before if (uwrap.initialised) statement which can be passed by concurrent threads. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 401d92a4e3af5d754b01c91b88903c32b38b5be7 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 13:05:43 2014 +0200 uwrap: Fix race condition - glibc lookups. Patch adds libc_symbol_binding_mutex which guards global table of libc functions and their lookup. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 724409c5911a2b166d7faa479ec328e9c6e09220 Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 12:33:21 2014 +0200 uwrap: Add library constructor and move pthread_atfork inside. Library constructor is used for pthread_atfork call. Moved here because pthread_atfork is cumulative and should be called only once. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 973a784ab16afc9e42a9ae50e4ba4639b2310c6a Author: Robin Hack <hack.ro...@gmail.com> Date: Fri Sep 26 12:08:58 2014 +0200 uwrap: Use UWRAP_LOCK/UNLOCK macros instead of pthread_mutex_lock/unlock calls. New macros UWRAP_LOCK/UNLOCK has been created and all calls to pthread_mutex_lock/unlock has been replaced by these macros. Signed-off-by: Robin Hack <hack.ro...@gmail.com> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> ----------------------------------------------------------------------- Summary of changes: ConfigureChecks.cmake | 21 ++++ config.h.cmake | 2 + src/uid_wrapper.c | 211 +++++++++++++++++++++++++++---------- tests/test_glibc_thread_support.c | 141 ++++++++++++++++++++++--- 4 files changed, 304 insertions(+), 71 deletions(-) Changeset truncated at 500 lines: diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake index 899d905..26c2238 100644 --- a/ConfigureChecks.cmake +++ b/ConfigureChecks.cmake @@ -125,6 +125,15 @@ int main(void) { endif (LINUX) check_c_source_compiles(" +#include <stdbool.h> +int main(void) { + bool x; + bool *p_x = &x; + __atomic_load(p_x, &x, __ATOMIC_RELAXED); + return 0; +}" HAVE_GCC_ATOMIC_BUILTINS) + +check_c_source_compiles(" __thread int tls; int main(void) { @@ -132,6 +141,18 @@ int main(void) { }" HAVE_GCC_THREAD_LOCAL_STORAGE) check_c_source_compiles(" +void test_constructor_attribute(void) __attribute__ ((constructor)); + +void test_constructor_attribute(void) +{ + return; +} + +int main(void) { + return 0; +}" HAVE_CONSTRUCTOR_ATTRIBUTE) + +check_c_source_compiles(" void test_destructor_attribute(void) __attribute__ ((destructor)); void test_destructor_attribute(void) diff --git a/config.h.cmake b/config.h.cmake index 840b5d4..b2a03b2 100644 --- a/config.h.cmake +++ b/config.h.cmake @@ -57,6 +57,8 @@ #cmakedefine HAVE_LINUX_32BIT_SYSCALLS 1 #cmakedefine HAVE_GCC_THREAD_LOCAL_STORAGE 1 +#cmakedefine HAVE_GCC_ATOMIC_BUILTINS 1 +#cmakedefine HAVE_CONSTRUCTOR_ATTRIBUTE 1 #cmakedefine HAVE_DESTRUCTOR_ATTRIBUTE 1 #cmakedefine HAVE_FUNCTION_ATTRIBUTE_FORMAT 1 diff --git a/src/uid_wrapper.c b/src/uid_wrapper.c index c1dc56b..8e3a7d3 100644 --- a/src/uid_wrapper.c +++ b/src/uid_wrapper.c @@ -43,6 +43,20 @@ # define UWRAP_THREAD #endif +# define UWRAP_LOCK(m) do { \ + pthread_mutex_lock(&( m ## _mutex)); \ +} while(0) + +# define UWRAP_UNLOCK(m) do { \ + pthread_mutex_unlock(&( m ## _mutex)); \ +} while(0) + +#ifdef HAVE_CONSTRUCTOR_ATTRIBUTE +#define CONSTRUCTOR_ATTRIBUTE __attribute__ ((constructor)) +#else +#define CONSTRUCTOR_ATTRIBUTE +#endif /* HAVE_CONSTRUCTOR_ATTRIBUTE */ + #ifdef HAVE_DESTRUCTOR_ATTRIBUTE #define DESTRUCTOR_ATTRIBUTE __attribute__ ((destructor)) #else @@ -222,8 +236,17 @@ struct uwrap { bool initialised; bool enabled; + uid_t ruid; + uid_t euid; + uid_t suid; + + gid_t rgid; + gid_t egid; + gid_t sgid; + + /* Real uid and gid of user who run uid wrapper */ uid_t myuid; - uid_t mygid; + gid_t mygid; struct uwrap_thread *ids; }; @@ -236,11 +259,15 @@ static UWRAP_THREAD struct uwrap_thread *uwrap_tls_id; /* The mutex or accessing the id */ static pthread_mutex_t uwrap_id_mutex = PTHREAD_MUTEX_INITIALIZER; +/* The mutex for accessing the global libc.fns */ +static pthread_mutex_t libc_symbol_binding_mutex = PTHREAD_MUTEX_INITIALIZER; + /********************************************************* * UWRAP PROTOTYPES *********************************************************/ bool uid_wrapper_enabled(void); +void uwrap_constructor(void) CONSTRUCTOR_ATTRIBUTE; void uwrap_destructor(void) DESTRUCTOR_ATTRIBUTE; /********************************************************* @@ -319,10 +346,12 @@ static void *_uwrap_load_lib_function(enum uwrap_lib lib, const char *fn_name) } #define uwrap_load_lib_function(lib, fn_name) \ + UWRAP_LOCK(libc_symbol_binding); \ if (uwrap.libc.fns._libc_##fn_name == NULL) { \ *(void **) (&uwrap.libc.fns._libc_##fn_name) = \ _uwrap_load_lib_function(lib, #fn_name); \ - } + } \ + UWRAP_UNLOCK(libc_symbol_binding) /* * IMPORTANT @@ -513,8 +542,13 @@ static int uwrap_new_id(pthread_t tid, bool do_alloc) id->tid = tid; id->dead = false; - id->ruid = id->euid = id->suid = uwrap.myuid; - id->rgid = id->egid = id->sgid = uwrap.mygid; + id->ruid = uwrap.ruid; + id->euid = uwrap.euid; + id->suid = uwrap.suid; + + id->rgid = uwrap.rgid; + id->egid = uwrap.egid; + id->sgid = uwrap.sgid; id->ngroups = 1; id->groups[0] = uwrap.mygid; @@ -524,8 +558,8 @@ static int uwrap_new_id(pthread_t tid, bool do_alloc) static void uwrap_thread_prepare(void) { - pthread_mutex_lock(&uwrap_id_mutex); - + UWRAP_LOCK(uwrap_id); + UWRAP_LOCK(libc_symbol_binding); /* * What happens if another atfork prepare functions calls a uwrap * function? So disable it in case another atfork prepare function @@ -538,32 +572,33 @@ static void uwrap_thread_parent(void) { uwrap.enabled = true; - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(libc_symbol_binding); + UWRAP_UNLOCK(uwrap_id); } static void uwrap_thread_child(void) { uwrap.enabled = true; - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(libc_symbol_binding); + UWRAP_UNLOCK(uwrap_id); } static void uwrap_init(void) { - const char *env = getenv("UID_WRAPPER"); + const char *env; pthread_t tid = pthread_self(); - - + UWRAP_LOCK(uwrap_id); if (uwrap.initialised) { struct uwrap_thread *id = uwrap_tls_id; int rc; if (id != NULL) { + UWRAP_UNLOCK(uwrap_id); return; } - pthread_mutex_lock(&uwrap_id_mutex); id = find_uwrap_id(tid); if (id == NULL) { rc = uwrap_new_id(tid, true); @@ -576,38 +611,30 @@ static void uwrap_init(void) uwrap_new_id(tid, false); } - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); return; } UWRAP_LOG(UWRAP_LOG_DEBUG, "Initialize uid_wrapper"); - /* - * If we hold a lock and the application forks, then the child - * is not able to unlock the mutex and we are in a deadlock. - * This should prevent such deadlocks. - */ - pthread_atfork(&uwrap_thread_prepare, - &uwrap_thread_parent, - &uwrap_thread_child); - - pthread_mutex_lock(&uwrap_id_mutex); - uwrap.initialised = true; uwrap.enabled = false; + env = getenv("UID_WRAPPER"); if (env != NULL && env[0] == '1') { const char *root = getenv("UID_WRAPPER_ROOT"); int rc; + uwrap.myuid = libc_geteuid(); + uwrap.mygid = libc_getegid(); /* put us in one group */ if (root != NULL && root[0] == '1') { - uwrap.myuid = 0; - uwrap.mygid = 0; + uwrap.ruid = uwrap.euid = uwrap.suid = 0; + uwrap.rgid = uwrap.egid = uwrap.sgid = 0; } else { - uwrap.myuid = libc_geteuid(); - uwrap.mygid = libc_getegid(); + uwrap.ruid = uwrap.euid = uwrap.suid = libc_geteuid(); + uwrap.rgid = uwrap.egid = uwrap.sgid = libc_getegid(); } rc = uwrap_new_id(tid, true); @@ -622,16 +649,22 @@ static void uwrap_init(void) uwrap.myuid == 0 ? "root" : "user"); } - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); UWRAP_LOG(UWRAP_LOG_DEBUG, "Succeccfully initialized uid_wrapper"); } bool uid_wrapper_enabled(void) { - uwrap_init(); - - return uwrap.enabled ? true : false; + bool enabled = false; + #ifdef HAVE_GCC_ATOMIC_BUILTINS + __atomic_load(&uwrap.enabled, &enabled, __ATOMIC_RELAXED); + #else + UWRAP_LOCK(uwrap_id); + enabled = uwrap.enabled; + UWRAP_UNLOCK(uwrap_id); + #endif + return enabled; } static int uwrap_setresuid_thread(uid_t ruid, uid_t euid, uid_t suid) @@ -643,7 +676,7 @@ static int uwrap_setresuid_thread(uid_t ruid, uid_t euid, uid_t suid) return -1; } - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); if (ruid != (uid_t)-1) { id->ruid = ruid; } @@ -655,7 +688,7 @@ static int uwrap_setresuid_thread(uid_t ruid, uid_t euid, uid_t suid) if (suid != (uid_t)-1) { id->suid = suid; } - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); return 0; } @@ -669,7 +702,7 @@ static int uwrap_setresuid(uid_t ruid, uid_t euid, uid_t suid) return -1; } - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); for (id = uwrap.ids; id; id = id->next) { if (id->dead) { continue; @@ -687,7 +720,21 @@ static int uwrap_setresuid(uid_t ruid, uid_t euid, uid_t suid) id->suid = suid; } } - pthread_mutex_unlock(&uwrap_id_mutex); + + /* Reflect changes in thread to main process. */ + if (ruid != (uid_t)-1) { + uwrap.ruid = ruid; + } + + if (euid != (uid_t)-1) { + uwrap.euid = euid; + } + + if (suid != (uid_t)-1) { + uwrap.suid = suid; + } + + UWRAP_UNLOCK(uwrap_id); return 0; } @@ -701,6 +748,7 @@ int setuid(uid_t uid) return libc_setuid(uid); } + uwrap_init(); return uwrap_setresuid(uid, -1, -1); } @@ -716,6 +764,7 @@ int seteuid(uid_t euid) return libc_seteuid(euid); } + uwrap_init(); return uwrap_setresuid(-1, euid, -1); } #endif @@ -732,6 +781,7 @@ int setreuid(uid_t ruid, uid_t euid) return libc_setreuid(ruid, euid); } + uwrap_init(); return uwrap_setresuid(ruid, euid, -1); } #endif @@ -743,6 +793,7 @@ int setresuid(uid_t ruid, uid_t euid, uid_t suid) return libc_setresuid(ruid, euid, suid); } + uwrap_init(); return uwrap_setresuid(ruid, euid, suid); } #endif @@ -755,9 +806,9 @@ static uid_t uwrap_getuid(void) struct uwrap_thread *id = uwrap_tls_id; uid_t uid; - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); uid = id->ruid; - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); return uid; } @@ -768,6 +819,7 @@ uid_t getuid(void) return libc_getuid(); } + uwrap_init(); return uwrap_getuid(); } @@ -780,9 +832,9 @@ static uid_t uwrap_geteuid(void) struct uwrap_thread *id = uwrap_tls_id; uid_t uid; - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); uid = id->euid; - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); /* Disable root and return myuid */ if (env != NULL && env[0] == '1') { @@ -798,6 +850,7 @@ uid_t geteuid(void) return libc_geteuid(); } + uwrap_init(); return uwrap_geteuid(); } @@ -810,7 +863,7 @@ static int uwrap_setresgid_thread(gid_t rgid, gid_t egid, gid_t sgid) return -1; } - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); if (rgid != (gid_t)-1) { id->rgid = rgid; } @@ -822,7 +875,7 @@ static int uwrap_setresgid_thread(gid_t rgid, gid_t egid, gid_t sgid) if (sgid != (gid_t)-1) { id->sgid = sgid; } - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); return 0; } @@ -836,7 +889,7 @@ static int uwrap_setresgid(gid_t rgid, gid_t egid, gid_t sgid) return -1; } - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); for (id = uwrap.ids; id; id = id->next) { if (id->dead) { continue; @@ -854,7 +907,20 @@ static int uwrap_setresgid(gid_t rgid, gid_t egid, gid_t sgid) id->sgid = sgid; } } - pthread_mutex_unlock(&uwrap_id_mutex); + + /* Reflect changes in thread to main process. */ + if (rgid != (gid_t)-1) { + uwrap.rgid = rgid; + } + + if (egid != (gid_t)-1) { + uwrap.egid = egid; + } + + if (sgid != (gid_t)-1) { + uwrap.sgid = sgid; + } + UWRAP_UNLOCK(uwrap_id); return 0; } @@ -868,6 +934,7 @@ int setgid(gid_t gid) return libc_setgid(gid); } + uwrap_init(); return uwrap_setresgid(gid, -1, -1); } @@ -878,6 +945,7 @@ int setegid(gid_t egid) return libc_setegid(egid); } + uwrap_init(); return uwrap_setresgid(-1, egid, -1); } #endif @@ -889,6 +957,7 @@ int setregid(gid_t rgid, gid_t egid) return libc_setregid(rgid, egid); } + uwrap_init(); return uwrap_setresgid(rgid, egid, -1); } #endif @@ -900,6 +969,7 @@ int setresgid(gid_t rgid, gid_t egid, gid_t sgid) return libc_setresgid(rgid, egid, sgid); } + uwrap_init(); return uwrap_setresgid(rgid, egid, sgid); } #endif @@ -912,9 +982,9 @@ static gid_t uwrap_getgid(void) struct uwrap_thread *id = uwrap_tls_id; gid_t gid; - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); gid = id->rgid; - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); return gid; } @@ -925,6 +995,7 @@ gid_t getgid(void) return libc_getgid(); } + uwrap_init(); return uwrap_getgid(); } @@ -936,9 +1007,9 @@ static uid_t uwrap_getegid(void) struct uwrap_thread *id = uwrap_tls_id; gid_t gid; - pthread_mutex_lock(&uwrap_id_mutex); + UWRAP_LOCK(uwrap_id); gid = id->egid; - pthread_mutex_unlock(&uwrap_id_mutex); + UWRAP_UNLOCK(uwrap_id); -- UID Wrapper Repository