Re: pending patches?

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 11/13/2009 11:35 AM:
> I also discovered that
> FreeBSD getgroups(-1,NULL) fails with EFAULT instead of EINVAL, which
> means that it mistakenly populates a valid pointer rather than rejecting a
> negative argument; should be easy to fix today.

Like so.  And I feel silly for the buffer overrun in the test; I was
originally using xnmalloc, but didn't want drag in "xalloc.h" for the
test, but converted it wrong.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkr9uykACgkQ84KuGfSFAYBcLQCeL237kwCuLDKIjAjESC7WmH6t
3oAAoLKKUsZ9B7Tj25Yp8dQdM6JP4FfW
=nuzD
-----END PGP SIGNATURE-----

>From 1e642be936b3a09f6200d753f035ce045d9c5dac Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 13 Nov 2009 12:53:17 -0700
Subject: [PATCH] getgroups: work around FreeBSD bug

FreeBSD 7.2 mistakenly succeeds on getgroups(-1,ptr) (POSIX
requires EINVAL failure since -1 is less than the proper result).

* lib/getgroups.c (rpl_getgroups): Work around the bug.
* m4/getgroups.m4 (gl_FUNC_GETGROUPS): Detect the bug.
* doc/posix-functions/getgroups.texi (getgroups): Document it.
* tests/test-getgroups.c (main): Fix buffer overrun.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                          |    6 ++++++
 doc/posix-functions/getgroups.texi |    6 +++++-
 lib/getgroups.c                    |   16 ++++++++++------
 m4/getgroups.m4                    |   19 ++++++++++++++++++-
 tests/test-getgroups.c             |    2 +-
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 44e2120..b0b3016 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-11-13  Eric Blake  <address@hidden>

+       getgroups: work around FreeBSD bug
+       * lib/getgroups.c (rpl_getgroups): Work around the bug.
+       * m4/getgroups.m4 (gl_FUNC_GETGROUPS): Detect the bug.
+       * doc/posix-functions/getgroups.texi (getgroups): Document it.
+       * tests/test-getgroups.c (main): Fix buffer overrun.
+
        getgroups: avoid compilation failure
        * lib/getgroups.c (includes): Include <stdint.h> for SIZE_MAX.
        * modules/getgroups (Depends-on): Add stdint.
diff --git a/doc/posix-functions/getgroups.texi 
b/doc/posix-functions/getgroups.texi
index 745210e..a8478eb 100644
--- a/doc/posix-functions/getgroups.texi
+++ b/doc/posix-functions/getgroups.texi
@@ -12,7 +12,11 @@ getgroups
 This function is missing on some platforms:
 mingw.
 @item
-On Ultrix 4.3, @code{getgroups (0, 0)} always fails.  See macro
+On some platforms, this function fails to reject a negative count,
+even though that is less than the size that would be returned:
+FreeBSD 7.2.
address@hidden
+On Ultrix 4.3, @code{getgroups (0, NULL)} always fails.  See macro
 @samp{AC_FUNC_GETGROUPS}.
 @item
 On very old systems, this function operated on an array of @samp{int},
diff --git a/lib/getgroups.c b/lib/getgroups.c
index bb2b38d..c07f2de 100644
--- a/lib/getgroups.c
+++ b/lib/getgroups.c
@@ -40,6 +40,9 @@ getgroups (int n _UNUSED_PARAMETER_, GETGROUPS_T *groups 
_UNUSED_PARAMETER_)
 #else /* HAVE_GETGROUPS */

 # undef getgroups
+# ifndef GETGROUPS_ZERO_BUG
+#  define GETGROUPS_ZERO_BUG 0
+# endif

 /* On at least Ultrix 4.3 and NextStep 3.2, getgroups (0, NULL) always
    fails.  On other systems, it returns the number of supplemental
@@ -55,18 +58,19 @@ rpl_getgroups (int n, gid_t *group)
   GETGROUPS_T *gbuf;
   int saved_errno;

-  if (n != 0)
+  if (n < 0)
+    {
+      errno = EINVAL;
+      return -1;
+    }
+
+  if (n != 0 || !GETGROUPS_ZERO_BUG)
     {
       int result;
       int saved_errno;
       if (sizeof *group == sizeof *gbuf)
         return getgroups (n, (GETGROUPS_T *) group);

-      if (n < 0)
-        {
-          errno = EINVAL;
-          return -1;
-        }
       if (SIZE_MAX / sizeof *gbuf <= n)
         {
           errno = ENOMEM;
diff --git a/m4/getgroups.m4 b/m4/getgroups.m4
index 4d96712..0f8e89e 100644
--- a/m4/getgroups.m4
+++ b/m4/getgroups.m4
@@ -1,4 +1,4 @@
-# serial 14
+# serial 15

 dnl From Jim Meyering.
 dnl A wrapper around AC_FUNC_GETGROUPS.
@@ -21,6 +21,23 @@ AC_DEFUN([gl_FUNC_GETGROUPS],
   then
     AC_LIBOBJ([getgroups])
     REPLACE_GETGROUPS=1
+    AC_DEFINE([GETGROUPS_ZERO_BUG], [1], [Define this to 1 if
+      getgroups(0,NULL) does not return the number of groups.])
+  else
+    dnl Detect FreeBSD bug; POSIX requires getgroups(-1,ptr) to fail.
+    AC_CACHE_CHECK([whether getgroups handles negative values],
+      [gl_cv_func_getgroups_works],
+      [AC_RUN_IFELSE([AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
+        [[int size = getgroups (0, 0);
+          gid_t *list = malloc (size * sizeof *list);
+          return getgroups (-1, list) != -1;]])],
+        [gl_cv_func_getgroups_works=yes],
+        [gl_cv_func_getgroups_works=no],
+        [gl_cv_func_getgroups_works="guessing no"])])
+    if test "$gl_cv_func_getgroups_works" != yes; then
+      AC_LIBOBJ([getgroups])
+      REPLACE_GETGROUPS=1
+    fi
   fi
   test -n "$GETGROUPS_LIB" && LIBS="$GETGROUPS_LIB $LIBS"
 ])
diff --git a/tests/test-getgroups.c b/tests/test-getgroups.c
index b2e65a0..aed80f1 100644
--- a/tests/test-getgroups.c
+++ b/tests/test-getgroups.c
@@ -52,7 +52,7 @@ main (int argc, char **argv _UNUSED_PARAMETER_)
     }
   ASSERT (0 <= result);
   ASSERT (result + 1 < SIZE_MAX / sizeof *groups);
-  groups = malloc (result + 1 * sizeof *groups);
+  groups = malloc ((result + 1) * sizeof *groups);
   ASSERT (groups);
   groups[result] = -1;
   /* Check for EINVAL handling.  Not all processes have supplemental
-- 
1.6.5.rc1