gistfile1.diff

From 4b957521ad1f4f24c1b722db8244266271cf0a23 Mon Sep 17 00:00:00 2001
From: Natanael Copa <[email protected]>
Date: Mon, 1 Sep 2014 21:01:03 +0200
Subject: [PATCH] Use threadsafe wrapper for getpwnam/getgrnam

Even if rlm_unix is marked as RLM_TYPE_THREAD_UNSAFE, it runs in a
separate thread than the main thread. Both main thread and rlm_unix
uses thread unsafe getpwnam/getgrnam which causes segfault when under
stress.

We create a thread safe wrapper for those that uses TLS.
---
 src/include/libradius.h                           |   2 +
 src/lib/misc.c                                    | 144 ++++++++++++++++++++++
 src/main/command.c                                |   4 +-
 src/modules/rlm_detail/rlm_detail.c               |   2 +-
 src/modules/rlm_opendirectory/rlm_opendirectory.c |   6 +-
 src/modules/rlm_unix/rlm_unix.c                   |   6 +-
 6 files changed, 155 insertions(+), 9 deletions(-)

diff --git a/src/include/libradius.h b/src/include/libradius.h
index 6037672..cbb48a9 100644
--- a/src/include/libradius.h
+++ b/src/include/libradius.h
@@ -683,6 +683,8 @@ int64_t		fr_pow(int32_t base, uint8_t exp);
 int		fr_get_time(char const *date_str, time_t *date);
 int8_t		fr_pointer_cmp(void const *a, void const *b);
 void		fr_quick_sort(void const *to_sort[], int min_idx, int max_idx, fr_cmp_t cmp);
+struct passwd	*fr_getpwnam(const char *name);
+struct group	*fr_getgrnam(const char *name);
 /*
  *	Define TALLOC_DEBUG to check overflows with talloc.
  *	we can't use valgrind, because the memory used by
diff --git a/src/lib/misc.c b/src/lib/misc.c
index 93499b9..ac6eaee 100644
--- a/src/lib/misc.c
+++ b/src/lib/misc.c
@@ -27,6 +27,9 @@ RCSID("$Id$")
 #include	<ctype.h>
 #include	<sys/file.h>
 #include	<fcntl.h>
+#include	<unistd.h>
+#include	<pwd.h>
+#include	<grp.h>
 
 #define FR_PUT_LE16(a, val)\
 	do {\
@@ -50,7 +53,18 @@ static char const *months[] = {
 	"jan", "feb", "mar", "apr", "may", "jun",
 	"jul", "aug", "sep", "oct", "nov", "dec" };
 
+struct pwgrnam_buffer {
+	struct passwd pwd;
+	char *pwbuffer;
+	int pwsize;
+
+	struct group grp;
+	char *grbuffer;
+	int grsize;
+};
+
 fr_thread_local_setup(char *, fr_inet_ntop_buffer);	/* macro */
+fr_thread_local_setup(struct pwgrnam_buffer *, fr_pwgrnam_buffer); /* macro */
 
 typedef struct fr_talloc_link {
 	bool armed;
@@ -1715,3 +1729,133 @@ void fr_talloc_verify_cb(UNUSED const void *ptr, UNUSED int depth,
 	/* do nothing */
 }
 #endif
+
+/*
+ *	Explicitly cleanup the memory allocated to the pwgrnam
+ *	buffer.
+ */
+static void _fr_pwgrnam_free(void *arg)
+{
+	struct pwgrnam_buffer *p = (struct pwgrnam_buffer *)arg;
+	free(p->pwbuffer);
+	free(p->grbuffer);
+	free(p);
+}
+
+/*
+ *	Allocate buffers for our getpwnam/getgrnam wrappers.
+ */
+static struct pwgrnam_buffer *init_pwgrnam_buffer(void) {
+	struct pwgrnam_buffer *p;
+	int ret;
+
+	p = fr_thread_local_init(fr_pwgrnam_buffer, _fr_pwgrnam_free);
+	if (p)
+		return p;
+
+	p = malloc(sizeof(struct pwgrnam_buffer));
+	if (!p) {
+		fr_perror("Failed allocating pwnam/grnam buffer");
+		return NULL;
+	}
+
+#ifdef _SC_GETPW_R_SIZE_MAX
+	p->pwsize = sysconf(_SC_GETPW_R_SIZE_MAX);
+	if (p->pwsize <= 0)
+#endif
+		p->pwsize = 16384;
+
+#ifdef _SC_GETGR_R_SIZE_MAX
+	p->grsize = sysconf(_SC_GETGR_R_SIZE_MAX);
+	if (p->grsize <= 0)
+#endif
+		p->grsize = 16384;
+
+	p->pwbuffer = malloc(p->pwsize);
+	if (!p->pwbuffer) {
+		fr_perror("Failed allocating pwnam buffer");
+		free(p);
+		return NULL;
+	}
+
+	p->grbuffer = malloc(p->grsize);
+	if (!p->grbuffer) {
+		fr_perror("Failed allocating grnam buffer");
+		free(p->pwbuffer);
+		free(p);
+		return NULL;
+	}
+
+	ret = fr_thread_local_set(fr_pwgrnam_buffer, p);
+	if (ret != 0) {
+		fr_perror("Failed setting up TLS for pwnam buffer: %s", fr_syserror(ret));
+		_fr_pwgrnam_free(p);
+		return NULL;
+	}
+
+	return p;
+}
+
+/** Wrapper around getpwnam, search user database for a name
+ *
+ * getpwnam is not threadsafe so provide a thread-safe variant that
+ * uses TLS.
+ *
+ * @param name then username to search for
+ * @return NULL on error or not found, else pointer to thread local struct passwd buffer
+ */
+struct passwd *fr_getpwnam(const char *name)
+{
+	struct pwgrnam_buffer *p;
+	struct passwd *result;
+	int ret;
+
+	p = init_pwgrnam_buffer();
+	if (!p)
+		return NULL;
+
+	while ((ret = getpwnam_r(name, &p->pwd, p->pwbuffer, p->pwsize, &result)) == ERANGE) {
+		char *tmp = realloc(p->pwbuffer, p->pwsize * 2);
+		if (!tmp) {
+			fr_perror("Failed reallocating pwnam buffer");
+			return NULL;
+		}
+		p->pwsize *= 2;
+		p->pwbuffer = tmp;
+	}
+	if (ret < 0 || result == NULL)
+		return NULL;
+	return result;
+}
+
+/** Wrapper around getgrnam, search group database for a name
+ *
+ * getgrnam is not threadsafe so provide a thread-safe variant that
+ * uses TLS.
+ *
+ * @param name the name to search for
+ * @return NULL on error or not found, else pointer to thread local struct group buffer
+ */
+struct group *fr_getgrnam(const char *name)
+{
+	struct pwgrnam_buffer *p;
+	struct group *result;
+	int ret;
+
+	p = init_pwgrnam_buffer();
+	if (!p)
+		return NULL;
+
+	while ((ret = getgrnam_r(name, &p->grp, p->grbuffer, p->grsize, &result)) == ERANGE) {
+		char *tmp = realloc(p->grbuffer, p->grsize * 2);
+		if (!tmp) {
+			fr_perror("Failed reallocating pwnam buffer");
+			return NULL;
+		}
+		p->grsize *= 2;
+		p->grbuffer = tmp;
+	}
+	if (ret < 0 || result == NULL)
+		return NULL;
+	return result;
+}
diff --git a/src/main/command.c b/src/main/command.c
index ba25f43..7718cc1 100644
--- a/src/main/command.c
+++ b/src/main/command.c
@@ -2112,7 +2112,7 @@ static int command_socket_parse_unix(CONF_SECTION *cs, rad_listen_t *this)
 	if (sock->uid_name) {
 		struct passwd *pw;
 
-		pw = getpwnam(sock->uid_name);
+		pw = fr_getpwnam(sock->uid_name);
 		if (!pw) {
 			ERROR("Failed getting uid for %s: %s", sock->uid_name, fr_syserror(errno));
 			return -1;
@@ -2126,7 +2126,7 @@ static int command_socket_parse_unix(CONF_SECTION *cs, rad_listen_t *this)
 	if (sock->gid_name) {
 		struct group *gr;
 
-		gr = getgrnam(sock->gid_name);
+		gr = fr_getgrnam(sock->gid_name);
 		if (!gr) {
 			ERROR("Failed getting gid for %s: %s", sock->gid_name, fr_syserror(errno));
 			return -1;
diff --git a/src/modules/rlm_detail/rlm_detail.c b/src/modules/rlm_detail/rlm_detail.c
index bd3dd5f..d91cb87 100644
--- a/src/modules/rlm_detail/rlm_detail.c
+++ b/src/modules/rlm_detail/rlm_detail.c
@@ -375,7 +375,7 @@ static rlm_rcode_t CC_HINT(nonnull) detail_do(void *instance, REQUEST *request,
 	if (inst->group != NULL) {
 		gid = strtol(inst->group, &endptr, 10);
 		if (*endptr != '\0') {
-			grp = getgrnam(inst->group);
+			grp = fr_getgrnam(inst->group);
 			if (!grp) {
 				RDEBUG2("Unable to find system group '%s'", inst->group);
 				goto skip_group;
diff --git a/src/modules/rlm_opendirectory/rlm_opendirectory.c b/src/modules/rlm_opendirectory/rlm_opendirectory.c
index 39cc9e4..350c6db 100644
--- a/src/modules/rlm_opendirectory/rlm_opendirectory.c
+++ b/src/modules/rlm_opendirectory/rlm_opendirectory.c
@@ -359,7 +359,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(UNUSED void *instance, REQUEST
 
 	/* resolve SACL */
 	uuid_clear(guid_sacl);
-	groupdata = getgrnam(kRadiusSACLName);
+	groupdata = fr_getgrnam(kRadiusSACLName);
 	if (groupdata != NULL) {
 		err = mbr_gid_to_uuid(groupdata->gr_gid, guid_sacl);
 		if (err != 0) {
@@ -384,7 +384,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(UNUSED void *instance, REQUEST
 		 */
 		if (uuid_parse(rad_client->community, guid_nasgroup) != 0) {
 			/* attempt to resolve the name */
-			groupdata = getgrnam(rad_client->community);
+			groupdata = fr_getgrnam(rad_client->community);
 			if (!groupdata) {
 				AUTH("rlm_opendirectory: The group \"%s\" does not exist on this system.", rad_client->community);
 				return RLM_MODULE_FAIL;
@@ -423,7 +423,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(UNUSED void *instance, REQUEST
 	/* resolve user */
 	uuid_clear(uuid);
 
-	userdata = getpwnam(request->username->vp_strvalue);
+	userdata = fr_getpwnam(request->username->vp_strvalue);
 	if (userdata != NULL) {
 		err = mbr_uid_to_uuid(userdata->pw_uid, uuid);
 		if (err != 0)
diff --git a/src/modules/rlm_unix/rlm_unix.c b/src/modules/rlm_unix/rlm_unix.c
index 6f2a8a6..8c74340 100644
--- a/src/modules/rlm_unix/rlm_unix.c
+++ b/src/modules/rlm_unix/rlm_unix.c
@@ -89,11 +89,11 @@ static int groupcmp(UNUSED void *instance, REQUEST *req, UNUSED VALUE_PAIR *requ
 		return -1;
 	}
 
-	pwd = getpwnam(req->username->vp_strvalue);
+	pwd = fr_getpwnam(req->username->vp_strvalue);
 	if (!pwd)
 		return -1;
 
-	grp = getgrnam(check->vp_strvalue);
+	grp = fr_getgrnam(check->vp_strvalue);
 	if (!grp)
 		return -1;
 
@@ -198,7 +198,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(UNUSED void *instance, REQUEST
 		return RLM_MODULE_USERLOCK;
 	}
 #else /* OSFC2 */
-	if ((pwd = getpwnam(name)) == NULL) {
+	if ((pwd = fr_getpwnam(name)) == NULL) {
 		return RLM_MODULE_NOTFOUND;
 	}
 	encrypted_pass = pwd->pw_passwd;
-- 
2.1.0