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>
}
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) {