]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
xfs_quota: fix memory leak in quota_group_type() error path
authorEric Sandeen <sandeen@redhat.com>
Mon, 14 Apr 2014 06:12:43 +0000 (16:12 +1000)
committerDave Chinner <david@fromorbit.com>
Mon, 14 Apr 2014 06:12:43 +0000 (16:12 +1000)
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>
quota/quota.c

index 7e52ad2280d158bb5a59d356d24b0ef6fb37f90f..367da8ca16b179d35ab78528f5bfa6fbd636f4cf 100644 (file)
@@ -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) {