diff options
| -rw-r--r-- | NEWS | 3 | ||||
| -rw-r--r-- | TODO | 11 | ||||
| -rw-r--r-- | gl/lib/mgetgroups.c | 67 | ||||
| -rw-r--r-- | gl/m4/mgetgroups.m4 | 5 |
4 files changed, 64 insertions, 22 deletions
@@ -6,6 +6,9 @@ GNU coreutils NEWS -*- outline -*- configure --enable-no-install-program=groups now works. + id now uses getgrouplist, when possible. This results in + much better performance when there are many users and/or groups. + ls no longer segfaults on files in /proc when linked with an older version of libselinux. E.g., ls -l /proc/sys would dereference a NULL pointer. @@ -142,17 +142,6 @@ Add a distcheck-time test to ensure that every distributed file is either read-only(indicating generated) or is version-controlled and up to date. -Implement Ulrich Drepper's suggestion to use getgrouplist rather than - getugroups. This affects both `id' and `setuidgid', but makes a big - difference on systems with many users and/or groups, and makes id usable - once again on systems where access restrictions make getugroups fail. - But first we'll need a run-test (either in an autoconf macro or at - run time) to avoid the segfault bug in libc-2.3.2's getgrouplist. - In that case, we'd revert to using a new (to-be-written) getgrouplist - module that does most of what `id' already does. Or just avoid the - buggy use of getgrouplist by never passing it a buffer of length zero. - See http://bugzilla.redhat.com/200327 - remove `%s' notation (now that they're all gone, add a Makefile.maint sc_ rule to ensure no new ones are added): grep -E "\`%.{,4}s'" src/*.c diff --git a/gl/lib/mgetgroups.c b/gl/lib/mgetgroups.c index 6a4a42280..b63436abf 100644 --- a/gl/lib/mgetgroups.c +++ b/gl/lib/mgetgroups.c @@ -23,11 +23,27 @@ #include <unistd.h> #include <stdint.h> +#include <string.h> #include <errno.h> - +#if HAVE_GETGROUPLIST +#include <grp.h> +#endif #include "getugroups.h" #include "xalloc.h" + +static void * +allocate_groupbuf (int size) +{ + if (xalloc_oversized (size, sizeof (GETGROUPS_T))) + { + errno = ENOMEM; + return NULL; + } + + return malloc (size * sizeof (GETGROUPS_T)); +} + /* Like getugroups, but store the result in malloc'd storage. Set *GROUPS to the malloc'd list of all group IDs of which USERNAME is a member. If GID is not -1, store it first. GID should be the @@ -37,12 +53,51 @@ the number of groups. */ int -mgetgroups (const char *username, gid_t gid, GETGROUPS_T **groups) +mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) { int max_n_groups; int ng; GETGROUPS_T *g; +#if HAVE_GETGROUPLIST + /* We prefer to use getgrouplist if available, because it has better + performance characteristics. + + In glibc 2.3.2, getgrouplist is buggy. If you pass a zero as the + size of the output buffer, getgrouplist will still write to the + buffer. Contrary to what some versions of the getgrouplist + manpage say, this doesn't happen with nonzero buffer sizes. + Therefore our usage here just avoids a zero sized buffer. */ + if (username) + { + enum { INITIAL_GROUP_BUFSIZE = 1u }; + /* INITIAL_GROUP_BUFSIZE is initially small to ensure good test coverage */ + GETGROUPS_T smallbuf[INITIAL_GROUP_BUFSIZE]; + + max_n_groups = INITIAL_GROUP_BUFSIZE; + ng = getgrouplist (username, gid, smallbuf, &max_n_groups); + + g = allocate_groupbuf (max_n_groups); + if (g == NULL) + return -1; + + *groups = g; + if (INITIAL_GROUP_BUFSIZE < max_n_groups) + { + return getgrouplist (username, gid, g, &max_n_groups); + /* XXX: Ignoring the race with group size increase */ + } + else + { + /* smallbuf was big enough, so we already have our data */ + memcpy (g, smallbuf, max_n_groups * sizeof *g); + return 0; + } + /* getgrouplist failed, fall through and use getugroups instead. */ + } + /* else no username, so fall through and use getgroups. */ +#endif + max_n_groups = (username ? getugroups (0, NULL, username, gid) : getgroups (0, NULL)); @@ -52,13 +107,7 @@ mgetgroups (const char *username, gid_t gid, GETGROUPS_T **groups) if (max_n_groups < 0) max_n_groups = 5; - if (xalloc_oversized (max_n_groups, sizeof *g)) - { - errno = ENOMEM; - return -1; - } - - g = malloc (max_n_groups * sizeof *g); + g = allocate_groupbuf (max_n_groups); if (g == NULL) return -1; diff --git a/gl/m4/mgetgroups.m4 b/gl/m4/mgetgroups.m4 index 81835415f..da557311b 100644 --- a/gl/m4/mgetgroups.m4 +++ b/gl/m4/mgetgroups.m4 @@ -1,10 +1,11 @@ -#serial 1 -dnl Copyright (C) 2007 Free Software Foundation, Inc. +#serial 2 +dnl Copyright (C) 2007-2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_MGETGROUPS], [ + AC_CHECK_FUNCS(getgrouplist) AC_LIBOBJ([mgetgroups]) ]) |