[SCM] UID Wrapper Repository - branch master updated

Andreas Schneider asn at samba.org
Fri Jul 21 15:20:35 UTC 2017


The branch, master has been updated
       via  4ec0dfc uwrap: Add a workaround for glibc malloc mutex deadlock bug
       via  d178906 uwrap: First do garbage collection before exporting ids
       via  0bfd224 tests: Add setgroups to test_fork_exec
       via  2fae075 uwrap: Improve groups string creation
       via  e242f39 uwrap: Make the unsigned_str smaller
       via  c1a6e70 uwrap: Fix type of len value
       via  f1d0330 uwrap: Fix two error messages
       via  61139e0 uwrap: Fix a typo
       via  6568b53 uwrap: Fix a deadlock if uid_wrapper is not enabled
       via  8fb7450 uwrap: Fix logging on optimized build
      from  7386dc6 Bump version to 1.2.3

https://git.samba.org/?p=uid_wrapper.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 4ec0dfc252961c411f8770560236a3c27596b3ad
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jul 21 10:54:05 2017 +0200

    uwrap: Add a workaround for glibc malloc mutex deadlock bug
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit d178906bcd1f1e2a067e7ddfdf82d24cb49128c7
Author: Andreas Schneider <asn at cryptomilk.org>
Date:   Wed Jul 19 16:27:05 2017 +0200

    uwrap: First do garbage collection before exporting ids
    
    Signed-off-by: Andreas Schneider <asn at cryptomilk.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 0bfd22468247105523a171ffbbe8678de07b1a55
Author: Andreas Schneider <asn at cryptomilk.org>
Date:   Wed Jul 19 16:22:39 2017 +0200

    tests: Add setgroups to test_fork_exec
    
    Signed-off-by: Andreas Schneider <asn at cryptomilk.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 2fae075fd0b1a3aef2041118e16755edbedc3a90
Author: Andreas Schneider <asn at cryptomilk.org>
Date:   Wed Jul 19 15:57:34 2017 +0200

    uwrap: Improve groups string creation
    
    Signed-off-by: Andreas Schneider <asn at cryptomilk.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit e242f39a5abf93d9b4b6ffbef1b808e2926739a2
Author: Andreas Schneider <asn at cryptomilk.org>
Date:   Wed Jul 19 15:44:32 2017 +0200

    uwrap: Make the unsigned_str smaller
    
    Signed-off-by: Andreas Schneider <asn at cryptomilk.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit c1a6e70d62aa14efab4bf41ce2230b76aa455807
Author: Andreas Schneider <asn at cryptomilk.org>
Date:   Wed Jul 19 15:43:13 2017 +0200

    uwrap: Fix type of len value
    
    snprintf return an 'int'.
    
    Signed-off-by: Andreas Schneider <asn at cryptomilk.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit f1d03308d84944914318053314112cdfcc6222da
Author: Andreas Schneider <asn at samba.org>
Date:   Wed Jul 19 12:15:16 2017 +0200

    uwrap: Fix two error messages
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 61139e06f4479d41bed166a0b2b6185d50a11dd3
Author: Andreas Schneider <asn at samba.org>
Date:   Wed Jul 19 12:14:15 2017 +0200

    uwrap: Fix a typo
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 6568b531e58cb387a306c17f9c30ff599c30b81c
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jul 21 08:28:50 2017 +0200

    uwrap: Fix a deadlock if uid_wrapper is not enabled
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 8fb7450712e94a6e1ceaddab4ac2a4d9670d715a
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jul 21 11:39:59 2017 +0200

    uwrap: Fix logging on optimized build
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 src/uid_wrapper.c      | 92 ++++++++++++++++++++++++++++++++++----------------
 tests/test_fork_exec.c |  7 ++++
 2 files changed, 70 insertions(+), 29 deletions(-)


Changeset truncated at 500 lines:

diff --git a/src/uid_wrapper.c b/src/uid_wrapper.c
index 0e9be96..0d74d20 100644
--- a/src/uid_wrapper.c
+++ b/src/uid_wrapper.c
@@ -825,7 +825,7 @@ static void uwrap_export_ids(struct uwrap_thread *id)
 {
 	char groups_str[GROUP_STRING_SIZE] = {0};
 	size_t groups_str_size = sizeof(groups_str);
-	char unsigned_str[32] = {0};
+	char unsigned_str[16] = {0}; /* We need 10 + 1 (+ 1) */
 	int i;
 
 	/* UIDS */
@@ -851,35 +851,51 @@ static void uwrap_export_ids(struct uwrap_thread *id)
 	if (id->ngroups > GROUP_MAX_COUNT) {
 		UWRAP_LOG(UWRAP_LOG_ERROR,
 			  "ERROR: Number of groups (%u) exceeds maximum value "
-			  "uid_wrapper will handle (%u).",
+			  "uid_wrapper can handle (%u).",
 			  id->ngroups,
 			  GROUP_MAX_COUNT);
 		exit(-1);
 	}
 
 	/* GROUPS */
-	snprintf(unsigned_str, sizeof(unsigned_str), "%u", id->ngroups);
-	setenv("UID_WRAPPER_INITIAL_GROUPS_COUNT", unsigned_str, 1);
-
 	for (i = 0; i < id->ngroups; i++) {
 		size_t groups_str_len = strlen(groups_str);
-		size_t groups_str_avail = groups_str_size - groups_str_len;
-		size_t len;
+		size_t groups_str_avail = groups_str_size - groups_str_len - 1;
+		int len;
 
 		len = snprintf(unsigned_str, sizeof(unsigned_str), ",%u", id->groups[i]);
 		if (len <= 1) {
-			continue;
+			UWRAP_LOG(UWRAP_LOG_ERROR,
+				  "snprintf failed for groups[%d]=%u",
+				  i,
+				  id->groups[i]);
+			break;
+		}
+		if (((size_t)len) >= groups_str_avail) {
+			UWRAP_LOG(UWRAP_LOG_ERROR,
+				  "groups env string is to small for %d groups",
+				  i);
+			break;
 		}
-		if (len < groups_str_avail) {
-			snprintf(groups_str + groups_str_len,
-				 groups_str_size - groups_str_len,
-				 "%s",
-				 i == 0 ? unsigned_str + 1 : unsigned_str);
+
+		len = snprintf(groups_str + groups_str_len,
+			       groups_str_size - groups_str_len,
+			       "%s",
+			       i == 0 ? unsigned_str + 1 : unsigned_str);
+		if (len < 1) {
+			UWRAP_LOG(UWRAP_LOG_ERROR,
+				  "snprintf failed to create groups string at groups[%d]=%u",
+				  i,
+				  id->groups[i]);
+			break;
 		}
 	}
 
-	if (id->ngroups > 0) {
+	if (id->ngroups == i) {
 		setenv("UID_WRAPPER_INITIAL_GROUPS", groups_str, 1);
+
+		snprintf(unsigned_str, sizeof(unsigned_str), "%u", id->ngroups);
+		setenv("UID_WRAPPER_INITIAL_GROUPS_COUNT", unsigned_str, 1);
 	}
 }
 
@@ -887,13 +903,13 @@ static void uwrap_thread_prepare(void)
 {
 	struct uwrap_thread *id = uwrap_tls_id;
 
+	UWRAP_LOCK_ALL;
+
 	/* uid_wrapper is loaded but not enabled */
 	if (id == NULL) {
 		return;
 	}
 
-	UWRAP_LOCK_ALL;
-
 	/*
 	 * What happens if another atfork prepare functions calls a uwrap
 	 * function? So disable it in case another atfork prepare function
@@ -909,6 +925,7 @@ static void uwrap_thread_parent(void)
 
 	/* uid_wrapper is loaded but not enabled */
 	if (id == NULL) {
+		UWRAP_UNLOCK_ALL;
 		return;
 	}
 
@@ -924,11 +941,10 @@ static void uwrap_thread_child(void)
 
 	/* uid_wrapper is loaded but not enabled */
 	if (id == NULL) {
+		UWRAP_UNLOCK_ALL;
 		return;
 	}
 
-	uwrap_export_ids(id);
-
 	/*
 	 * "Garbage collector" - Inspired by DESTRUCTOR.
 	 * All threads (except one which called fork()) are dead now.. Dave
@@ -949,6 +965,8 @@ static void uwrap_thread_child(void)
 		u = uwrap.ids;
 	}
 
+	uwrap_export_ids(id);
+
 	id->enabled = true;
 
 	UWRAP_UNLOCK_ALL;
@@ -1048,8 +1066,8 @@ static void uwrap_init_env(struct uwrap_thread *id)
 		if (i != ngroups) {
 			UWRAP_LOG(UWRAP_LOG_ERROR,
 				  "ERROR: The number of groups (%u) passed, "
-				  "does not the number of groups (%u) we "
-				  "parsed",
+				  "does not match the number of groups (%u) "
+				  "we parsed.",
 				  ngroups,
 				  i);
 			exit(-1);
@@ -1160,7 +1178,7 @@ static void uwrap_init(void)
 
 	UWRAP_UNLOCK(uwrap_id);
 
-	UWRAP_LOG(UWRAP_LOG_DEBUG, "Succeccfully initialized uid_wrapper");
+	UWRAP_LOG(UWRAP_LOG_DEBUG, "Successfully initialized uid_wrapper");
 }
 
 bool uid_wrapper_enabled(void)
@@ -1333,9 +1351,7 @@ static int uwrap_setreuid_args(uid_t ruid, uid_t euid,
 
 static int uwrap_setreuid_thread(uid_t ruid, uid_t euid)
 {
-#ifndef NDEBUG
 	struct uwrap_thread *id = uwrap_tls_id;
-#endif
 	uid_t new_ruid = -1, new_euid = -1, new_suid = -1;
 	int rc;
 
@@ -1354,9 +1370,7 @@ static int uwrap_setreuid_thread(uid_t ruid, uid_t euid)
 #ifdef HAVE_SETREUID
 static int uwrap_setreuid(uid_t ruid, uid_t euid)
 {
-#ifndef NDEBUG
 	struct uwrap_thread *id = uwrap_tls_id;
-#endif
 	uid_t new_ruid = -1, new_euid = -1, new_suid = -1;
 	int rc;
 
@@ -1620,9 +1634,7 @@ static int uwrap_setregid_args(gid_t rgid, gid_t egid,
 
 static int uwrap_setregid_thread(gid_t rgid, gid_t egid)
 {
-#ifndef NDEBUG
 	struct uwrap_thread *id = uwrap_tls_id;
-#endif
 	gid_t new_rgid = -1, new_egid = -1, new_sgid = -1;
 	int rc;
 
@@ -1641,9 +1653,7 @@ static int uwrap_setregid_thread(gid_t rgid, gid_t egid)
 #ifdef HAVE_SETREGID
 static int uwrap_setregid(gid_t rgid, gid_t egid)
 {
-#ifndef NDEBUG
 	struct uwrap_thread *id = uwrap_tls_id;
-#endif
 	gid_t new_rgid = -1, new_egid = -1, new_sgid = -1;
 	int rc;
 
@@ -2281,8 +2291,30 @@ long int syscall (long int sysno, ...)
 /****************************
  * CONSTRUCTOR
  ***************************/
+
 void uwrap_constructor(void)
 {
+	char *glibc_malloc_lock_bug;
+
+	/*
+	 * This is a workaround for a bug in glibc < 2.24:
+	 *
+	 * The child handler for the malloc() function is called and locks the
+	 * mutex. Then our child handler is called and we try to call setenv().
+	 * setenv() wants to malloc and tries to aquire the lock for malloc and
+	 * we end up in a deadlock.
+	 *
+	 * So as a workaround we need to call malloc once before we setup the
+	 * handlers.
+	 *
+	 * See https://sourceware.org/bugzilla/show_bug.cgi?id=16742
+	 */
+	glibc_malloc_lock_bug = malloc(1);
+	if (glibc_malloc_lock_bug == NULL) {
+		exit(-1);
+	}
+	glibc_malloc_lock_bug[0] = '\0';
+
 	/*
 	* 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.
@@ -2292,6 +2324,8 @@ void uwrap_constructor(void)
 		       &uwrap_thread_parent,
 		       &uwrap_thread_child);
 
+	free(glibc_malloc_lock_bug);
+
 	/* Here is safe place to call uwrap_init() and initialize data
 	 * for main process.
 	 */
diff --git a/tests/test_fork_exec.c b/tests/test_fork_exec.c
index 36077c4..177f321 100644
--- a/tests/test_fork_exec.c
+++ b/tests/test_fork_exec.c
@@ -6,12 +6,15 @@
 #include <cmocka.h>
 
 #include <errno.h>
+#include <grp.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
 
+#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
+
 #define TEST_MOCK_EXECUTABLE "mock_exec_uid"
 
 static void test_uwrap_fork_exec(void **state)
@@ -19,6 +22,7 @@ static void test_uwrap_fork_exec(void **state)
 	pid_t pid;
 	int rc;
 	uid_t cp_ruid, cp_euid, cp_suid;
+	gid_t glist[] = {0, 10000, 20000, 40000, 100000};
 
 	(void)state; /* unused */
 
@@ -34,6 +38,9 @@ static void test_uwrap_fork_exec(void **state)
 	rc = setgid(5000);
 	assert_return_code(rc, errno);
 
+	rc = setgroups(ARRAY_SIZE(glist), glist);
+	assert_return_code(rc, errno);
+
 	pid = fork();
 	assert_return_code(pid, errno);
 


-- 
UID Wrapper Repository



More information about the samba-cvs mailing list