summaryrefslogtreecommitdiffstats
path: root/libglusterfs
diff options
context:
space:
mode:
authorCsaba Henk <csaba@redhat.com>2017-07-05 17:48:37 +0200
committerJeff Darcy <jeff@pl.atyp.us>2017-07-06 18:26:08 +0000
commite14ea3f5c37475e12a3b7fb7bd3165b0a4e77c51 (patch)
tree7ef467ec7fa1761379a101ada475f504c479a42f /libglusterfs
parentfc73ae5f81ef5926e3dc2311db116250d0f2a321 (diff)
groups: don't allocate auxiliary gid list on stack
When glusterfs wants to retrieve the list of auxiliary gids of a user, it typically allocates a sufficiently big gid_t array on stack and calls getgrouplist(3) with it. However, "sufficiently big" means to be of maximum supported gid list size, which in GlusterFS is GF_MAX_AUX_GROUPS = 64k. That means a 64k * sizeof(gid_t) = 256k allocation, which is big enough to overflow the stack in certain cases. A further observation is that stack allocation of the gid list brings no gain, as in all cases the content of the gid list eventually gets copied over to a heap allocated buffer. So we add a convenience wrapper of getgrouplist to libglusterfs called gf_getgrouplist which calls getgrouplist with a sufficiently big heap allocated buffer (it takes care of the allocation too). We are porting all the getgrouplist invocations to gf_getgrouplist and thus eliminate the huge stack allocation. BUG: 1464327 Change-Id: Icea76d0d74dcf2f87d26cb299acc771ca3b32d2b Signed-off-by: Csaba Henk <csaba@redhat.com> Reviewed-on: https://review.gluster.org/17706 Smoke: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Niels de Vos <ndevos@redhat.com> Reviewed-by: Amar Tumballi <amarts@redhat.com> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Diffstat (limited to 'libglusterfs')
-rw-r--r--libglusterfs/src/common-utils.c73
-rw-r--r--libglusterfs/src/common-utils.h3
-rw-r--r--libglusterfs/src/glusterfs.h3
-rw-r--r--libglusterfs/src/stack.h15
4 files changed, 86 insertions, 8 deletions
diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 5015d9666b6..e5e08909ac7 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -30,6 +30,7 @@
#include <signal.h>
#include <assert.h>
#include <libgen.h> /* for dirname() */
+#include <grp.h>
#if defined(GF_BSD_HOST_OS) || defined(GF_DARWIN_HOST_OS)
#include <sys/sysctl.h>
@@ -4877,3 +4878,75 @@ close_fds_except (int *fdv, size_t count)
#endif /* !GF_LINUX_HOST_OS */
return 0;
}
+
+/**
+ * gf_getgrouplist - get list of groups to which a user belongs
+ *
+ * A convenience wrapper for getgrouplist(3).
+ *
+ * @param user - same as in getgrouplist(3)
+ * @param group - same as in getgrouplist(3)
+ * @param groups - pointer to a gid_t pointer
+ *
+ * gf_getgrouplist allocates a gid_t buffer which is big enough to
+ * hold the list of auxiliary group ids for user, up to the GF_MAX_AUX_GROUPS
+ * threshold. Upon succesfull invocation groups will be pointed to that buffer.
+ *
+ * @return success: the number of auxiliary group ids retrieved
+ * failure: -1
+ */
+int
+gf_getgrouplist (const char *user, gid_t group, gid_t **groups)
+{
+ int ret = -1;
+ int ngroups = SMALL_GROUP_COUNT;
+
+ *groups = GF_CALLOC (sizeof (gid_t), ngroups, gf_common_mt_groups_t);
+ if (!*groups)
+ return -1;
+
+ /*
+ * We are running getgrouplist() in a loop until we succeed (or hit
+ * certain exit conditions, see the comments below). This is because
+ * the indicated number of auxiliary groups that we obtain in case of
+ * the failure of the first invocation is not guaranteed to keep its
+ * validity upon the next invocation with a gid buffer of that size.
+ */
+ for (;;) {
+ int ngroups_old = ngroups;
+ ret = getgrouplist (user, group, *groups, &ngroups);
+ if (ret != -1)
+ break;
+
+ if (ngroups >= GF_MAX_AUX_GROUPS) {
+ /*
+ * This should not happen as GF_MAX_AUX_GROUPS is set
+ * to the max value of number of supported auxiliary
+ * groups across all platforms supported by GlusterFS.
+ * However, if it still happened some way, we wouldn't
+ * care about the incompleteness of the result, we'd
+ * just go on with what we got.
+ */
+ return GF_MAX_AUX_GROUPS;
+ } else if (ngroups <= ngroups_old) {
+ /*
+ * There is an edge case that getgrouplist() fails but
+ * ngroups remains the same. This is actually not
+ * specified in getgrouplist(3), but implementations
+ * can do this upon internal failure[1]. To avoid
+ * falling into an infinite loop when this happens, we
+ * break the loop if the getgrouplist call failed
+ * without an increase in the indicated group number.
+ *
+ * [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=grp/initgroups.c;hb=refs/heads/release/2.25/master#l168
+ */
+ GF_FREE (*groups);
+ return -1;
+ }
+
+ *groups = GF_REALLOC (*groups, ngroups * sizeof (gid_t));
+ if (!*groups)
+ return -1;
+ }
+ return ret;
+}
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index c3c5ec77350..6a260c090c8 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -918,4 +918,7 @@ get_ip_from_addrinfo (struct addrinfo *addr, char **ip);
int
close_fds_except (int *fdv, size_t count);
+
+int
+gf_getgrouplist (const char *user, gid_t group, gid_t **groups);
#endif /* _COMMON_UTILS_H */
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index ee5c21d43d5..2cd1f363b3b 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -235,9 +235,6 @@
#define GF_REPLICATE_TRASH_DIR ".landfill"
/* GlusterFS's maximum supported Auxiliary GIDs */
-/* TODO: Keeping it to 200, so that we can fit in 2KB buffer for auth data
- * in RPC server code, if there is ever need for having more aux-gids, then
- * we have to add aux-gid in payload of actors */
#define GF_MAX_AUX_GROUPS 65535
#define GF_UUID_BUF_SIZE 50
diff --git a/libglusterfs/src/stack.h b/libglusterfs/src/stack.h
index 20fbdfabff5..eb5848e92aa 100644
--- a/libglusterfs/src/stack.h
+++ b/libglusterfs/src/stack.h
@@ -357,21 +357,26 @@ STACK_RESET (call_stack_t *stack)
} while (0)
+static void
+call_stack_set_groups (call_stack_t *stack, int ngrps, gid_t *groupbuf)
+{
+ stack->groups = groupbuf;
+ stack->ngrps = ngrps;
+}
+
static inline int
call_stack_alloc_groups (call_stack_t *stack, int ngrps)
{
if (ngrps <= SMALL_GROUP_COUNT) {
- stack->groups = stack->groups_small;
+ call_stack_set_groups (stack, ngrps, stack->groups_small);
} else {
- stack->groups_large = GF_CALLOC (sizeof (gid_t), ngrps,
+ stack->groups_large = GF_CALLOC (ngrps, sizeof (gid_t),
gf_common_mt_groups_t);
if (!stack->groups_large)
return -1;
- stack->groups = stack->groups_large;
+ call_stack_set_groups (stack, ngrps, stack->groups_large);
}
- stack->ngrps = ngrps;
-
return 0;
}