bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Problem with id when mgetgroups returns no groups.


From: Eric Blake
Subject: Re: Problem with id when mgetgroups returns no groups.
Date: Fri, 04 Dec 2009 07:01:37 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.23) Gecko/20090812 Thunderbird/2.0.0.23 Mnenhy/0.7.6.666

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

[adding gnulib, since mgetgroups now lives in gnulib]

According to address@hidden on 12/4/2009 3:55 AM:
> All,
> 
> In coreutils 8.1, src/id.c line 296:
> 
>      gid_t *groups;
>      int i;
> 
>      int n_groups = mgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1),
>                                          &groups);

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.

>      if (n_groups < 0 && errno != ENOSYS)

One failure case is indeed errno==ENOSYS.  (By the way, which system did
you find this on?)

>      free (groups);

Which then does indeed reach this point with uninitialized memory.

> if mgetgroups doesn't find any groups, "groups" will not be changed and
> therefore still contain an uninitialised value which is later freed on
> the last line of this extract.  The fix we have here is to initialise
> groups to NULL, then test before we free (although with glibc that isn't
> actually necessary).

Actually, C requires that free(NULL) works, and gnulib guarantees that for
coreutils.  In fact, coreutils has a syntax check that rejects attempt to
check for NULL prior to calling free.

> Thought you might like to know.

I'd like to push this as the fix.  However, it would be nicer to credit a
full name, rather than just "Scott", for listing you in THANKS.

- --
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/

iEYEARECAAYFAksZFkEACgkQ84KuGfSFAYDq+QCeNPQzQAC1VIsP5xqnWvRQDLO6
kegAn2RLfg5tKYul3vDrgRq2Lxa3EQWf
=c7BS
-----END PGP SIGNATURE-----
>From 04e2e65befd34b56d8c25ff1508f4a928165fd4b Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 4 Dec 2009 06:58:56 -0700
Subject: [PATCH] id: fix bad free on systems without getgroups support

* src/id.c (print_full_info): Initialize pointer, since mgetgroups
does not touch it when failing with ENOSYS.
* THANKS: Update.
Reported by Scott.
---
 src/id.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/id.c b/src/id.c
index 9a00f5c..ec16cd2 100644
--- a/src/id.c
+++ b/src/id.c
@@ -293,7 +293,7 @@ print_full_info (const char *username)
     }

   {
-    gid_t *groups;
+    gid_t *groups = NULL;
     int i;

     int n_groups = mgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1),
-- 
1.6.5.rc1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]