Eric Blake <ebb9 <at> byu.net> writes: > Thanks for your report. Indeed, looking at mgetgroups.c, if getgroups > fails, mgetgroups returns -1 without assigning through *groups (likewise > if realloc_groupbug fails, but that sets errno to ENOMEM). I also wonder > if mgetgroups should be taught to recognize ENOSYS itself, and guarantee a > zero-length array rather than returning with -1 in that case.
I spotted another problem - id was not handling ENOMEM failures consistently (in other words, it didn't go through xalloc_die). How about this alternative patch, which makes gnulib aware of ENOSYS, and changes coreutils to use the more robust interface? From: Eric Blake <[email protected]> Date: Fri, 4 Dec 2009 08:26:23 -0700 Subject: [PATCH] mgetgroups: add xgetgroups, and avoid ENOSYS failures * lib/mgetgroups.h (xgetgroups): New prototype. * lib/mgetgroups.c (xgetgroups): New wrapper. (mgetgroups): Handle ENOSYS. * modules/mgetgroups (Depends-on): Add realloc-posix. Reported by Scott <scott.gnu.2009 AT scottrix.co.uk>. Signed-off-by: Eric Blake <[email protected]> --- ChangeLog | 8 ++++++++ lib/mgetgroups.c | 27 ++++++++++++++++++++++++--- lib/mgetgroups.h | 1 + modules/mgetgroups | 1 + 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index f913f41..319f3c1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2009-12-04 Eric Blake <[email protected]> + + mgetgroups: add xgetgroups, and avoid ENOSYS failures + * lib/mgetgroups.h (xgetgroups): New prototype. + * lib/mgetgroups.c (xgetgroups): New wrapper. + (mgetgroups): Handle ENOSYS. + * modules/mgetgroups (Depends-on): Add realloc-posix. + 2009-11-17 Eric Blake <[email protected]> manywarnings: add more warnings diff --git a/lib/mgetgroups.c b/lib/mgetgroups.c index f68e28f..3c5ec8d 100644 --- a/lib/mgetgroups.c +++ b/lib/mgetgroups.c @@ -118,10 +118,19 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) ? getugroups (0, NULL, username, gid) : getgroups (0, NULL) + (gid != -1)); - /* If we failed to count groups with NULL for a buffer, - try again with a non-NULL one, just in case. */ + /* If we failed to count groups because there is no supplemental + group support, then return an array containing just GID. + Otherwise, we fail for the same reason. */ if (max_n_groups < 0) - max_n_groups = 5; + { + if (errno == ENOSYS && (g = realloc_groupbuf (NULL, 1))) + { + *groups = g; + *g = gid; + return gid != -1; + } + return -1; + } g = realloc_groupbuf (NULL, max_n_groups); if (g == NULL) @@ -133,6 +142,7 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) if (ng < 0) { + /* Failure is unexpected, but handle it anyway. */ int saved_errno = errno; free (g); errno = saved_errno; @@ -147,3 +157,14 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) *groups = g; return ng; } + +/* Like mgetgroups, but call xalloc_die on allocation failure. */ + +int +xgetgroups (char const *username, gid_t gid, gid_t **groups) +{ + int result = mgetgroups (username, gid, groups); + if (result == -1 && errno == ENOMEM) + xalloc_die (); + return result; +} diff --git a/lib/mgetgroups.h b/lib/mgetgroups.h index 909d84c..4868d28 100644 --- a/lib/mgetgroups.h +++ b/lib/mgetgroups.h @@ -17,3 +17,4 @@ #include <sys/types.h> int mgetgroups (const char *username, gid_t gid, gid_t **groups); +int xgetgroups (const char *username, gid_t gid, gid_t **groups); diff --git a/modules/mgetgroups b/modules/mgetgroups index 58ef740..45ae412 100644 --- a/modules/mgetgroups +++ b/modules/mgetgroups @@ -9,6 +9,7 @@ m4/mgetgroups.m4 Depends-on: getgroups getugroups +realloc-posix xalloc configure.ac: -- 1.6.4.2 From: Eric Blake <[email protected]> Date: Fri, 4 Dec 2009 08:06:55 -0700 Subject: [PATCH] id: handle systems without getgroups support If getgroups failed with ENOSYS, mgetgroups would unnecessarily fail, and that provoked id into freeing an uninitialized pointer. Meanwhile, we were not using xalloc_die properly. Both issues are better solved in gnulib, by introducing xgetgroups; this patch uses the new interface. * gnulib: Update, for mgetgroups improvments. * src/id.c (print_full_info): Adjust caller to die on allocation failure, and no longer worry about ENOSYS. * src/group-list.c (print_group_list): Likewise. * src/setuidgid.c (main): Likewise. --- gnulib | 2 +- src/group-list.c | 4 ++-- src/id.c | 4 ++-- src/setuidgid.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gnulib b/gnulib index cdfe647..c5588be 160000 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit cdfe647f8d29540cdfe90cef0fa568c5d2fd4481 +Subproject commit 64a72bbf6ed20208d633b9e9d849000aee279a01 diff --git a/src/group-list.c b/src/group-list.c index 1fadd0c..a4b1f6d 100644 --- a/src/group-list.c +++ b/src/group-list.c @@ -58,9 +58,9 @@ print_group_list (const char *username, gid_t *groups; int i; - int n_groups = mgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1), + int n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1), &groups); - if (n_groups < 0 && errno != ENOSYS) + if (n_groups < 0) { if (username) { diff --git a/src/id.c b/src/id.c index 9a00f5c..96d8e96 100644 --- a/src/id.c +++ b/src/id.c @@ -296,9 +296,9 @@ print_full_info (const char *username) gid_t *groups; int i; - int n_groups = mgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1), + int n_groups = xgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1), &groups); - if (n_groups < 0 && errno != ENOSYS) + if (n_groups < 0) { if (username) { diff --git a/src/setuidgid.c b/src/setuidgid.c index 0adac21..2710bc9 100644 --- a/src/setuidgid.c +++ b/src/setuidgid.c @@ -179,7 +179,7 @@ main (int argc, char **argv) #if HAVE_SETGROUPS if (n_gids == 0) { - int n = mgetgroups (pwd->pw_name, pwd->pw_gid, &gids); + int n = xgetgroups (pwd->pw_name, pwd->pw_gid, &gids); if (n <= 0) error (EXIT_FAILURE, errno, _("failed to get groups for user %s"), quote (pwd->pw_name)); -- 1.6.4.2
