From: Eric Sandeen <sandeen@redhat.com>
Date: Mon, 14 Apr 2014 06:12:43 +0000 (+1000)
Subject: xfs_quota: fix memory leak in quota_group_type() error path
X-Git-Tag: v3.2.0-rc1~13
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f6a8b88b5d00289268b5c732d6716c58194fee8f;p=thirdparty%2Fxfsprogs-dev.git

xfs_quota: fix memory leak in quota_group_type() error path

quota_group_type has some rather contorted logic that's
been around since 2005.

In the (!name) case, if any of the 3 calls setting up ngroups fails,
we fall back to using just one group.

However, if it's the getgroups() call that fails, we overwrite
the allocated gid ptr with &gid, thus leaking that allocated
memory.  Worse, we set "dofree" to 1, so will free non-allocated
local var gid.  And that last else case is redundant; if we get there,
gids is guaranteed to be non-null.

Refactor it a bit to be more clear (I hope) and correct.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---

diff --git a/quota/quota.c b/quota/quota.c
index 7e52ad228..367da8ca1 100644
--- a/quota/quota.c
+++ b/quota/quota.c
@@ -289,15 +289,19 @@ quota_group_type(
 		}
 		gids = &gid;
 		ngroups = 1;
-	} else if ( ((ngroups = sysconf(_SC_NGROUPS_MAX)) < 0) ||
-		    ((gids = malloc(ngroups * sizeof(gid_t))) == NULL) ||
-		    ((ngroups = getgroups(ngroups, gids)) < 0)) {
-		dofree = (gids != NULL);
-		gid = getgid();
-		gids = &gid;
-		ngroups = 1;
 	} else {
-		dofree = (gids != NULL);
+		if ( ((ngroups = sysconf(_SC_NGROUPS_MAX)) < 0) ||
+		     ((gids = malloc(ngroups * sizeof(gid_t))) == NULL) ||
+		     ((ngroups = getgroups(ngroups, gids)) < 0)) {
+			/* something failed.  Fall back to 1 group */
+			free(gids);
+			gid = getgid();
+			gids = &gid;
+			ngroups = 1;
+		} else {
+			/* It all worked, and we allocated memory */
+			dofree = 1;
+		}
 	}
 
 	for (i = 0; i < ngroups; i++, name = NULL) {