Steven Parkes wrote: > Very similar to my "correct" patch. There's one other issue I found when I > was coming up with a patch which is that Darwin:getgrouplist also doesn't > return the number of groups. It sets the in/out parameter, but then just > returns 0.
Right so your previous patch would have always returned 0 for users in more than 10 groups. I noticed that the docs don't actually say the number of groups is returned, and was going to ask you to test a new version returning max_n_groups rather than ng still works, as I thought it more robust. So you've already tested that and shown in fact that it's required. > The only thing I wasn't too sure about was how risky it is to just keep > doubling. If some bizzaro-world getgrouplist was just implemented to return > -1, well, the results wouldn’t be great. But maybe that's not a significant > concern. If there was an existing getgrouplist that always returned -1 then we should handle it. But that would be a very unlikely bug and if introduced in future would be noticed quickly and fixed at source. So I'm not sure about handling limits here. For reference some relevant limits on my 32 bit linux system. $ cat /proc/sys/kernel/overflowgid 65534 $ eval $(git/coreutils/src/getlimits); echo $GID_T_MAX 4294967295 Updated patch attached. cheers, Pádraig.
>From 47df4c1efcce97413786ef84452e5cfedcb56fc6 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Wed, 8 Apr 2009 10:43:15 +0100 Subject: [PATCH] id: fix infinite loop on some systems Steven Parkes reported that `id -G $USER` went into an infinite loop on Darwin systems for users in more than 10 groups: http://bugs.gentoo.org/show_bug.cgi?id=264007 * gl/lib/mgetgroups.c (mgetgroups): Work around buggy getgrouplist implementations that don't update the required size correctly, by doubling the result buffer and retrying. Also return the parameter updated by getgrouplist rather than its return value, as the documentation doesn't actually state the number of groups stored is returned by getgrouplist. * tests/misc/id-groups: Add test to exercise this logic * tests/Makefile.am: Reference new test * NEWS: Mention the fix * THANKS: Update --- NEWS | 5 +++++ THANKS | 1 + gl/lib/mgetgroups.c | 10 +++++++++- tests/Makefile.am | 1 + tests/misc/id-groups | 28 ++++++++++++++++++++++++++++ 5 files changed, 44 insertions(+), 1 deletions(-) create mode 100755 tests/misc/id-groups diff --git a/NEWS b/NEWS index de1db44..976e3a7 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,11 @@ GNU coreutils NEWS -*- outline -*- ls now aligns output correctly in the presence of abbreviated month names from the locale database that have differing widths. + `id -G $USER` now works correctly on Darwin and NetBSD. Previously it + would either truncate the group list to 10 or go into an infinite loop. + [truncation bug introduced in coreutils-6.11] + [infinite loop bug introduced in coreutils-7.1] + ** Changes in behavior shred, sort, shuf: now use an internal pseudorandom generator by default. diff --git a/THANKS b/THANKS index 6a918a4..fe523fe 100644 --- a/THANKS +++ b/THANKS @@ -525,6 +525,7 @@ Steve McIntyre [email protected] Steve Ward [email protected] Steven G. Johnson [email protected] Steven Mocking [email protected] +Steven Parkes [email protected] Steven Schveighoffer [email protected] Steven P Watson [email protected] Stuart Kemp [email protected] diff --git a/gl/lib/mgetgroups.c b/gl/lib/mgetgroups.c index e697013..ac1d398 100644 --- a/gl/lib/mgetgroups.c +++ b/gl/lib/mgetgroups.c @@ -81,10 +81,16 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) while (1) { GETGROUPS_T *h; + int last_n_groups = max_n_groups; /* getgrouplist updates max_n_groups to num required. */ ng = getgrouplist (username, gid, g, &max_n_groups); + /* Some systems (like Darwin) have a bug where they + never increase max_n_groups. */ + if ((0 > ng) && (last_n_groups == max_n_groups)) + max_n_groups *= 2; + if ((h = realloc_groupbuf (g, max_n_groups)) == NULL) { int saved_errno = errno; @@ -97,7 +103,9 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) if (0 <= ng) { *groups = g; - return ng; + /* Some systems just return 0 or -1 from getgrouplist, + so return max_n_groups rather than ng. */ + return max_n_groups; } } } diff --git a/tests/Makefile.am b/tests/Makefile.am index 07f34ec..8ce6a21 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -171,6 +171,7 @@ TESTS = \ misc/head-c \ misc/head-pos \ misc/id-context \ + misc/id-groups \ misc/md5sum \ misc/md5sum-newline \ misc/mknod \ diff --git a/tests/misc/id-groups b/tests/misc/id-groups new file mode 100755 index 0000000..dc0f54c --- /dev/null +++ b/tests/misc/id-groups @@ -0,0 +1,28 @@ +#!/bin/sh +# Ensure that "id" outputs groups for a user +# Copyright (C) 2009 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +if test "$VERBOSE" = yes; then + set -x + id --version +fi + +. $srcdir/test-lib.sh + +fail=0 +id -G $(id -nu) || fail=1 + +Exit $fail -- 1.5.3.6
_______________________________________________ Bug-coreutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-coreutils
