From ba4af70fb7d3ae21187ac0e9e3fe5539a4f8f829 Mon Sep 17 00:00:00 2001
From: Csaba Henk <csaba@redhat.com>
Date: Wed, 5 Jul 2017 17:48:37 +0200
Subject: [PATCH 545/557] 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
> 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>
BUG: 1452513
Change-Id: Icea76d0d74dcf2f87d26cb299acc771ca3b32d2b
Signed-off-by: Csaba Henk <csaba@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/111305
Tested-by: Csaba Henk <chenk@redhat.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
libglusterfs/src/common-utils.c | 73 ++++++++++++++++++++++++++++
libglusterfs/src/common-utils.h | 3 ++
libglusterfs/src/glusterfs.h | 3 --
libglusterfs/src/stack.h | 15 ++++--
xlators/mount/fuse/src/fuse-helpers.c | 20 +++-----
xlators/nfs/server/src/nfs-fops.c | 36 +++++---------
xlators/protocol/server/src/server-helpers.c | 23 +++------
7 files changed, 113 insertions(+), 60 deletions(-)
diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 862ec3b..c123f70 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>
@@ -4853,3 +4854,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 1af9529..4765b5e 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -903,4 +903,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 d812aa7..43acf4c 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -233,9 +233,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 be7e900..c2848b7 100644
--- a/libglusterfs/src/stack.h
+++ b/libglusterfs/src/stack.h
@@ -422,21 +422,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;
}
diff --git a/xlators/mount/fuse/src/fuse-helpers.c b/xlators/mount/fuse/src/fuse-helpers.c
index 3e54197..5ccb0a5 100644
--- a/xlators/mount/fuse/src/fuse-helpers.c
+++ b/xlators/mount/fuse/src/fuse-helpers.c
@@ -20,7 +20,6 @@
#include <sys/sysctl.h>
#endif
#include <pwd.h>
-#include <grp.h>
#include "fuse-bridge.h"
@@ -158,8 +157,8 @@ frame_fill_groups (call_frame_t *frame)
char *saveptr = NULL;
char *endptr = NULL;
int ret = 0;
- int ngroups = FUSE_MAX_AUX_GROUPS;
- gid_t mygroups[GF_MAX_AUX_GROUPS];
+ int ngroups = 0;
+ gid_t *mygroups = NULL;
if (priv->resolve_gids) {
struct passwd pwent;
@@ -173,23 +172,16 @@ frame_fill_groups (call_frame_t *frame)
return;
}
- ngroups = GF_MAX_AUX_GROUPS;
- if (getgrouplist (result->pw_name, frame->root->gid, mygroups,
- &ngroups) == -1) {
+ ngroups = gf_getgrouplist (result->pw_name, frame->root->gid,
+ &mygroups);
+ if (ngroups == -1) {
gf_log (this->name, GF_LOG_ERROR, "could not map %s to "
"group list (ngroups %d, max %d)",
result->pw_name, ngroups, GF_MAX_AUX_GROUPS);
return;
}
- if (call_stack_alloc_groups (frame->root, ngroups) != 0)
- goto out;
-
- /* Copy data to the frame. */
- for (idx = 0; idx < ngroups; ++idx) {
- frame->root->groups[idx] = mygroups[idx];
- }
- frame->root->ngrps = ngroups;
+ call_stack_set_groups (frame->root, ngroups, mygroups);
} else {
ret = snprintf (filename, sizeof filename, "/proc/%d/status",
frame->root->pid);
diff --git a/xlators/nfs/server/src/nfs-fops.c b/xlators/nfs/server/src/nfs-fops.c
index f6361f0..28b1781 100644
--- a/xlators/nfs/server/src/nfs-fops.c
+++ b/xlators/nfs/server/src/nfs-fops.c
@@ -8,7 +8,6 @@
cases as published by the Free Software Foundation.
*/
-#include <grp.h>
#include <pwd.h>
#include "dict.h"
@@ -34,12 +33,7 @@ nfs_fix_groups (xlator_t *this, call_stack_t *root)
struct passwd mypw;
char mystrs[1024];
struct passwd *result;
-#ifdef GF_DARWIN_HOST_OS
- /* BSD/DARWIN does not correctly uses gid_t in getgrouplist */
- int mygroups[GF_MAX_AUX_GROUPS];
-#else
- gid_t mygroups[GF_MAX_AUX_GROUPS];
-#endif
+ gid_t *mygroups;
int ngroups;
int i;
int max_groups;
@@ -88,25 +82,13 @@ nfs_fix_groups (xlator_t *this, call_stack_t *root)
gf_msg_trace (this->name, 0, "mapped %u => %s",
root->uid, result->pw_name);
- ngroups = GF_MAX_AUX_GROUPS;
- if (getgrouplist(result->pw_name,root->gid,mygroups,&ngroups) == -1) {
+ ngroups = gf_getgrouplist (result->pw_name, root->gid, &mygroups);
+ if (ngroups == -1) {
gf_msg (this->name, GF_LOG_ERROR, 0, NFS_MSG_MAP_GRP_LIST_FAIL,
"could not map %s to group list", result->pw_name);
return;
}
- /* Add the group data to the cache. */
- gl.gl_list = GF_CALLOC(ngroups, sizeof(gid_t), gf_nfs_mt_aux_gids);
- if (gl.gl_list) {
- /* It's not fatal if the alloc failed. */
- gl.gl_id = root->uid;
- gl.gl_uid = 0;
- gl.gl_gid = 0;
- gl.gl_count = ngroups;
- memcpy(gl.gl_list, mygroups, sizeof(gid_t) * ngroups);
- if (gid_cache_add(&priv->gid_cache, &gl) != 1)
- GF_FREE(gl.gl_list);
- }
/* RPC enforces the GF_AUTH_GLUSTERFS_MAX_GROUPS limit */
if (ngroups > max_groups) {
@@ -115,16 +97,24 @@ nfs_fix_groups (xlator_t *this, call_stack_t *root)
"too many groups, reducing %d -> %d",
ngroups, max_groups);
- ngroups = max_groups;
}
/* Copy data to the frame. */
- for (i = 0; i < ngroups; ++i) {
+ for (i = 0; i < ngroups && i < max_groups; ++i) {
gf_msg_trace (this->name, 0, "%s is in group %u",
result->pw_name, mygroups[i]);
root->groups[i] = mygroups[i];
}
root->ngrps = ngroups;
+
+ /* Add the group data to the cache. */
+ gl.gl_list = mygroups;
+ gl.gl_id = root->uid;
+ gl.gl_uid = 0;
+ gl.gl_gid = 0;
+ gl.gl_count = ngroups;
+ if (gid_cache_add(&priv->gid_cache, &gl) != 1)
+ GF_FREE(mygroups);
}
struct nfs_fop_local *
diff --git a/xlators/protocol/server/src/server-helpers.c b/xlators/protocol/server/src/server-helpers.c
index 590662b..3ad3d84 100644
--- a/xlators/protocol/server/src/server-helpers.c
+++ b/xlators/protocol/server/src/server-helpers.c
@@ -19,7 +19,6 @@
#include <fnmatch.h>
#include <pwd.h>
-#include <grp.h>
#include "compound-fop-utils.h"
/* based on nfs_fix_aux_groups() */
@@ -30,10 +29,10 @@ gid_resolve (server_conf_t *conf, call_stack_t *root)
struct passwd mypw;
char mystrs[1024];
struct passwd *result;
- gid_t mygroups[GF_MAX_AUX_GROUPS];
+ gid_t *mygroups;
gid_list_t gl;
const gid_list_t *agl;
- int ngroups, i;
+ int ngroups;
agl = gid_cache_lookup (&conf->gid_cache, root->uid, 0, 0);
if (agl) {
@@ -58,9 +57,8 @@ gid_resolve (server_conf_t *conf, call_stack_t *root)
gf_msg_trace ("gid-cache", 0, "mapped %u => %s", root->uid,
result->pw_name);
- ngroups = GF_MAX_AUX_GROUPS;
- ret = getgrouplist (result->pw_name, root->gid, mygroups, &ngroups);
- if (ret == -1) {
+ ngroups = gf_getgrouplist (result->pw_name, root->gid, &mygroups);
+ if (ngroups == -1) {
gf_msg ("gid-cache", GF_LOG_ERROR, 0, PS_MSG_MAPPING_ERROR,
"could not map %s to group list (%d gids)",
result->pw_name, root->ngrps);
@@ -84,8 +82,10 @@ fill_groups:
if (gl.gl_list)
memcpy (gl.gl_list, mygroups,
sizeof(gid_t) * root->ngrps);
- else
+ else {
+ GF_FREE (mygroups);
return -1;
+ }
}
if (root->ngrps == 0) {
@@ -93,14 +93,7 @@ fill_groups:
goto out;
}
- if (call_stack_alloc_groups (root, root->ngrps) != 0) {
- ret = -1;
- goto out;
- }
-
- /* finally fill the groups from the */
- for (i = 0; i < root->ngrps; ++i)
- root->groups[i] = gl.gl_list[i];
+ call_stack_set_groups (root, root->ngrps, mygroups);
out:
if (agl) {
--
1.8.3.1