Re: ls segfault - coreutils-6.5

[Jim Meyering]

Paul Eggert <address@hidden> wrote:
> Greg Schafer <address@hidden> writes:
>
>> Admnittedly, this is an unusual situation in that it's inside an almost
>> empty chroot environment before everything is sane ie: it's part of my
>> "bootstrap a whole new world" procedure. Here is a backtrace. Hope it helps:
>>
>> (gdb) bt
>> #0  0xb7e7ceb3 in strlen () from /temptools/lib/libc.so.6
>> #1  0x08053265 in getgroup (gid=0) at idcache.c:146
>
> Ouch, thanks for reporting that.  This bug is my fault; sorry.
> I didn't notice that the idcache code cached the absence of groups.
>
> I installed the following obvious quick patch into gnulib; it should
> fix your coreutils problem (apply just the idcache.c fix).
>
> 2006-11-20  Paul Eggert  <address@hidden>
>
>       * lib/idcache.c: Undo most recent patch, dated 2006-11-06.
>       It mishandled the case where the group was missing.
>       Problem reported by Greg Schafer.
>       * modules/idcache: Likewise.

FYI, I wanted to have my cake and eat it too, so was about to
check in the following change.  I'll merge things and make a
new coreutils release soon.

2006-11-20  Jim Meyering  <address@hidden>

        Don't dereference NULL.  Don't return the empty string.
        * lib/idcache.c: Remove all uses of "register" keyword.
        (getgroup): Don't dereference NULL.  Due to typo in 2006-11-06 change.
        (getgroup, getuser): When there is no match, return NULL, per the API,
        not the empty string.

Index: idcache.c
===================================================================
RCS file: /sources/gnulib/gnulib/lib/idcache.c,v
retrieving revision 1.18
diff -u -p -r1.18 idcache.c
--- idcache.c   6 Nov 2006 22:02:53 -0000       1.18
+++ idcache.c   20 Nov 2006 09:15:25 -0000
@@ -55,24 +55,32 @@ static struct userid *nouser_alist;
 char *
 getuser (uid_t uid)
 {
-  register struct userid *tail;
-  struct passwd *pwent;
-  char const *name;
+  struct userid *tail;
+  struct userid *match = NULL;

   for (tail = user_alist; tail; tail = tail->next)
-    if (tail->id.u == uid)
-      return tail->name;
+    {
+      if (tail->id.u == uid)
+       {
+         match = tail;
+         break;
+       }
+    }

-  pwent = getpwuid (uid);
-  name = pwent ? pwent->pw_name : "";
-  tail = xmalloc (offsetof (struct userid, name) + strlen (name) + 1);
-  tail->id.u = uid;
-  strcpy (tail->name, name);
+  if (match == NULL)
+    {
+      struct passwd *pwent = getpwuid (uid);
+      char const *name = pwent ? pwent->pw_name : "";
+      match = xmalloc (offsetof (struct userid, name) + strlen (name) + 1);
+      match->id.u = uid;
+      strcpy (match->name, name);
+
+      /* Add to the head of the list, so most recently used is first.  */
+      match->next = user_alist;
+      user_alist = match;
+    }

-  /* Add to the head of the list, so most recently used is first.  */
-  tail->next = user_alist;
-  user_alist = tail;
-  return tail->name;
+  return match->name[0] ? match->name : NULL;
 }

 /* Translate USER to a UID, with cache.
@@ -83,7 +91,7 @@ getuser (uid_t uid)
 uid_t *
 getuidbyname (const char *user)
 {
-  register struct userid *tail;
+  struct userid *tail;
   struct passwd *pwent;

   for (tail = user_alist; tail; tail = tail->next)
@@ -94,7 +102,7 @@ getuidbyname (const char *user)
   for (tail = nouser_alist; tail; tail = tail->next)
     /* Avoid a function call for the most common case.  */
     if (*tail->name == *user && !strcmp (tail->name, user))
-      return 0;
+      return NULL;

   pwent = getpwnam (user);
 #ifdef __DJGPP__
@@ -121,7 +129,7 @@ getuidbyname (const char *user)

   tail->next = nouser_alist;
   nouser_alist = tail;
-  return 0;
+  return NULL;
 }

 /* Use the same struct as for userids.  */
@@ -133,24 +141,32 @@ static struct userid *nogroup_alist;
 char *
 getgroup (gid_t gid)
 {
-  register struct userid *tail;
-  struct group *grent;
-  char const *name;
+  struct userid *tail;
+  struct userid *match = NULL;

   for (tail = group_alist; tail; tail = tail->next)
-    if (tail->id.g == gid)
-      return tail->name;
+    {
+      if (tail->id.g == gid)
+       {
+         match = tail;
+         break;
+       }
+    }

-  grent = getgrgid (gid);
-  name = grent ? grent->gr_name : NULL;
-  tail = xmalloc (offsetof (struct userid, name) + strlen (name) + 1);
-  tail->id.g = gid;
-  strcpy (tail->name, name);
+  if (match == NULL)
+    {
+      struct group *grent = getgrgid (gid);
+      char const *name = grent ? grent->gr_name : "";
+      match = xmalloc (offsetof (struct userid, name) + strlen (name) + 1);
+      match->id.g = gid;
+      strcpy (match->name, name);
+
+      /* Add to the head of the list, so most recently used is first.  */
+      match->next = group_alist;
+      group_alist = match;
+    }

-  /* Add to the head of the list, so most recently used is first.  */
-  tail->next = group_alist;
-  group_alist = tail;
-  return tail->name;
+  return match->name[0] ? match->name : NULL;
 }

 /* Translate GROUP to a GID, with cache.
@@ -161,7 +177,7 @@ getgroup (gid_t gid)
 gid_t *
 getgidbyname (const char *group)
 {
-  register struct userid *tail;
+  struct userid *tail;
   struct group *grent;

   for (tail = group_alist; tail; tail = tail->next)
@@ -172,12 +188,12 @@ getgidbyname (const char *group)
   for (tail = nogroup_alist; tail; tail = tail->next)
     /* Avoid a function call for the most common case.  */
     if (*tail->name == *group && !strcmp (tail->name, group))
-      return 0;
+      return NULL;

   grent = getgrnam (group);
 #ifdef __DJGPP__
   /* We need to pretend to belong to group GROUP, to make
-     grp functions know about any arbitrary group name.  */
+     grp functions know about an arbitrary group name.  */
   if (!grent && strspn (group, digits) < strlen (group))
     {
       setenv ("GROUP", group, 1);
@@ -199,5 +215,5 @@ getgidbyname (const char *group)

   tail->next = nogroup_alist;
   nogroup_alist = tail;
-  return 0;
+  return NULL;
 }