diff options
author | Csaba Henk <csaba@redhat.com> | 2017-07-05 17:48:37 +0200 |
---|---|---|
committer | Jeff Darcy <jeff@pl.atyp.us> | 2017-07-06 18:26:08 +0000 |
commit | e14ea3f5c37475e12a3b7fb7bd3165b0a4e77c51 (patch) | |
tree | 7ef467ec7fa1761379a101ada475f504c479a42f /xlators | |
parent | fc73ae5f81ef5926e3dc2311db116250d0f2a321 (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 'xlators')
-rw-r--r-- | xlators/mount/fuse/src/fuse-helpers.c | 20 | ||||
-rw-r--r-- | xlators/nfs/server/src/nfs-fops.c | 36 | ||||
-rw-r--r-- | xlators/protocol/server/src/server-helpers.c | 23 |
3 files changed, 27 insertions, 52 deletions
diff --git a/xlators/mount/fuse/src/fuse-helpers.c b/xlators/mount/fuse/src/fuse-helpers.c index b65e6ebfc7b..3fc6b16c41f 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 f6361f02161..28b1781f4b1 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 37f7bca47f0..09e0012d70a 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) { |