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