Subject: Re: bug#10639: BUG REPORT coreutils-8.15 Solaris 10 64bit

From: Paul Eggert <[email-masked]>
Date: Wed, 01 Feb 2012 01:14:10 -0800

On 01/30/2012 07:01 PM, JONES, BILL wrote:
> those are vxfs.

Thanks, can you please try the attached patch?
You can apply it by running the shell commands:

  cd lib
  patch < vxfs-patch.txt

I'll CC: this to bug-gnulib as it appears to be a Gnulib bug.

Thanks.
acl: fix infinite loop on Solaris 10 + vxfs
Problem reported by Bill Jones in
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10639>.
* lib/acl-internal.h: Include <limits.h>.
(MIN): New macro.
* lib/copy-acl.c (qcopy_acl): Don't object if we get fewer ACL
entries than we originally counted; perhaps some were removed
as we were running, or perhaps we can count but not get.
Check for size-calculation overflow.
Avoid need for dynamic allocation if possible.
* lib/file-has-acl.c (file_has_acl): Likewise.
* lib/set-mode-acl.c (qset_acl): Likewise.
diff --git a/lib/acl-internal.h b/lib/acl-internal.h
index 88e5e45..64701b2 100644
--- a/lib/acl-internal.h
+++ b/lib/acl-internal.h
@@ -55,6 +55,12 @@ extern int aclsort (int, int, struct acl *);
 # define ENOTSUP (-1)
 #endif

+#include <limits.h>
+
+#ifndef MIN
+# define MIN(a,b) ((a) < (b) ? (a) : (b))
+#endif
+
 #ifndef HAVE_FCHMOD
 # define HAVE_FCHMOD false
 # define fchmod(fd, mode) (-1)
diff --git a/lib/copy-acl.c b/lib/copy-acl.c
index 9b9f033..e9f4826 100644
--- a/lib/copy-acl.c
+++ b/lib/copy-acl.c
@@ -181,10 +181,10 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
      of Unixware.  The acl() call returns the access and default ACL both
      at once.  */
 # ifdef ACE_GETACL
-  int ace_count;
+  int ace_count0, ace_count;
   ace_t *ace_entries;
 # endif
-  int count;
+  int count0, count;
   aclent_t *entries;
   int did_chmod;
   int saved_errno;
@@ -228,19 +228,21 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
           break;
         }

-      ace_entries = (ace_t *) malloc (ace_count * sizeof (ace_t));
-      if (ace_entries == NULL)
+      if (! (ace_count < MIN (INT_MAX, (size_t) -1 / sizeof (ace_t))
+             && (ace_entries
+                 = (ace_t *) malloc ((ace_count + 1) * sizeof (ace_t)))))
         {
           errno = ENOMEM;
           return -2;
         }

-      if ((source_desc != -1
-           ? facl (source_desc, ACE_GETACL, ace_count, ace_entries)
-           : acl (src_name, ACE_GETACL, ace_count, ace_entries))
-          == ace_count)
+      ace_count0 = ace_count;
+      ace_count = (source_desc != -1
+                   ? facl (source_desc, ACE_GETACL, ace_count + 1, ace_entries)
+                   : acl (src_name, ACE_GETACL, ace_count + 1, ace_entries));
+      if (ace_count <= ace_count0)
         break;
-      /* Huh? The number of ACL entries changed since the last call.
+      /* Huh? The number of ACL entries grew since the last call.
          Repeat.  */
     }
 # endif
@@ -269,19 +271,21 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
           break;
         }

-      entries = (aclent_t *) malloc (count * sizeof (aclent_t));
-      if (entries == NULL)
+      if (! (count < MIN (INT_MAX, (size_t) -1 / sizeof (aclent_t))
+             && (entries
+                 = (aclent_t *) malloc ((count + 1) * sizeof (aclent_t)))))
         {
           errno = ENOMEM;
           return -2;
         }

-      if ((source_desc != -1
-           ? facl (source_desc, GETACL, count, entries)
-           : acl (src_name, GETACL, count, entries))
-          == count)
+      count0 = count;
+      count = (source_desc != -1
+               ? facl (source_desc, GETACL, count + 1, entries)
+               : acl (src_name, GETACL, count + 1, entries));
+      if (count <= count0)
         break;
-      /* Huh? The number of ACL entries changed since the last call.
+      /* Huh? The number of ACL entries grew since the last call.
          Repeat.  */
     }

@@ -366,77 +370,44 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
 #elif USE_ACL && HAVE_GETACL /* HP-UX */

   int count;
-  struct acl_entry entries[NACLENTRIES];
+  struct acl_entry entries[NACLENTRIES + 1];
 # if HAVE_ACLV_H
   int aclv_count;
-  struct acl aclv_entries[NACLVENTRIES];
+  struct acl aclv_entries[NACLVENTRIES + 1];
 # endif
   int did_chmod;
   int saved_errno;
   int ret;

-  for (;;)
-    {
-      count = (source_desc != -1
-               ? fgetacl (source_desc, 0, NULL)
-               : getacl (src_name, 0, NULL));
-
-      if (count < 0)
-        {
-          if (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTSUP)
-            {
-              count = 0;
-              break;
-            }
-          else
-            return -2;
-        }
+  count = (source_desc != -1
+           ? fgetacl (source_desc, NACLENTRIES + 1, entries)
+           : getacl (src_name, NACLENTRIES + 1, entries));

-      if (count == 0)
-        break;
-
-      if (count > NACLENTRIES)
-        /* If NACLENTRIES cannot be trusted, use dynamic memory allocation.  */
-        abort ();
-
-      if ((source_desc != -1
-           ? fgetacl (source_desc, count, entries)
-           : getacl (src_name, count, entries))
-          == count)
-        break;
-      /* Huh? The number of ACL entries changed since the last call.
-         Repeat.  */
+  if (count < 0)
+    {
+      if (! (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTSUP))
+        return -2;
+      count = 0;
     }

-# if HAVE_ACLV_H
-  for (;;)
-    {
-      aclv_count = acl ((char *) src_name, ACL_CNT, NACLVENTRIES, aclv_entries);
+  if (NACLENTRIES < count)
+    /* If NACLENTRIES cannot be trusted, use dynamic memory allocation.  */
+    abort ();

-      if (aclv_count < 0)
-        {
-          if (errno == ENOSYS || errno == EOPNOTSUPP || errno == EINVAL)
-            {
-              count = 0;
-              break;
-            }
-          else
-            return -2;
-        }
+# if HAVE_ACLV_H
+  aclv_count = acl ((char *) src_name, ACL_GET, NACLVENTRIES + 1, aclv_entries);

-      if (aclv_count == 0)
-        break;
+  if (aclv_count < 0)
+    {
+      if (! (errno == ENOSYS || errno == EOPNOTSUPP || errno == EINVAL))
+        return -2;
+      aclv_count = 0;
+    }

-      if (aclv_count > NACLVENTRIES)
-        /* If NACLVENTRIES cannot be trusted, use dynamic memory allocation.  */
-        abort ();
+  if (NACLVENTRIES < aclv_count)
+    /* If NACLVENTRIES cannot be trusted, use dynamic memory allocation.  */
+    abort ();

-      if (acl ((char *) src_name, ACL_GET, aclv_count, aclv_entries)
-          == aclv_count)
-        break;
-      /* Huh? The number of ACL entries changed since the last call.
-         Repeat.  */
-    }
 # endif

   if (count == 0)
@@ -546,37 +517,16 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,

 #elif USE_ACL && HAVE_ACLSORT /* NonStop Kernel */

-  int count;
-  struct acl entries[NACLENTRIES];
+  struct acl entries[NACLENTRIES + 1];
+  int count = acl ((char *) src_name, ACL_GET, NACLENTRIES + 1, entries);
   int ret;

-  for (;;)
-    {
-      count = acl ((char *) src_name, ACL_CNT, NACLENTRIES, NULL);
-
-      if (count < 0)
-        {
-          if (0)
-            {
-              count = 0;
-              break;
-            }
-          else
-            return -2;
-        }
-
-      if (count == 0)
-        break;
-
-      if (count > NACLENTRIES)
-        /* If NACLENTRIES cannot be trusted, use dynamic memory allocation.  */
-        abort ();
+  if (count < 0)
+    return -2;

-      if (acl ((char *) src_name, ACL_GET, count, entries) == count)
-        break;
-      /* Huh? The number of ACL entries changed since the last call.
-         Repeat.  */
-    }
+  if (count > NACLENTRIES)
+    /* If NACLENTRIES cannot be trusted, use dynamic memory allocation.  */
+    abort ();

   if (count == 0)
     return qset_acl (dst_name, dest_desc, mode);
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index b7c1484..ed187d2 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -532,107 +532,34 @@ file_has_acl (char const *name, struct stat const *sb)
       /* Solaris 2.5 through Solaris 10, Cygwin, and contemporaneous versions
          of Unixware.  The acl() call returns the access and default ACL both
          at once.  */
-      int count;
-      {
-        aclent_t *entries;
-
-        for (;;)
-          {
-            count = acl (name, GETACLCNT, 0, NULL);
-
-            if (count < 0)
-              {
-                if (errno == ENOSYS || errno == ENOTSUP)
-                  break;
-                else
-                  return -1;
-              }
-
-            if (count == 0)
-              break;
-
-            /* Don't use MIN_ACL_ENTRIES:  It's set to 4 on Cygwin, but Cygwin
-               returns only 3 entries for files with no ACL.  But this is safe:
-               If there are more than 4 entries, there cannot be only the
-               "user::", "group::", "other:", and "mask:" entries.  */
-            if (count > 4)
-              return 1;
-
-            entries = (aclent_t *) malloc (count * sizeof (aclent_t));
-            if (entries == NULL)
-              {
-                errno = ENOMEM;
-                return -1;
-              }
-            if (acl (name, GETACL, count, entries) == count)
-              {
-                if (acl_nontrivial (count, entries))
-                  {
-                    free (entries);
-                    return 1;
-                  }
-                free (entries);
-                break;
-              }
-            /* Huh? The number of ACL entries changed since the last call.
-               Repeat.  */
-            free (entries);
-          }
-      }
+      aclent_t entries[5];
+      int count = acl (name, GETACL, 5, entries);
+      if (0 <= count)
+        return acl_nontrivial (count, entries);
+      if (! (errno == ENOSYS || errno == ENOTSUP))
+        return -1;

 #   ifdef ACE_GETACL
+
       /* Solaris also has a different variant of ACLs, used in ZFS and NFSv4
-         file systems (whereas the other ones are used in UFS file systems).  */
-      {
-        ace_t *entries;
-
-        for (;;)
-          {
-            count = acl (name, ACE_GETACLCNT, 0, NULL);
-
-            if (count < 0)
-              {
-                if (errno == ENOSYS || errno == EINVAL)
-                  break;
-                else
-                  return -1;
-              }
-
-            if (count == 0)
-              break;
-
-            /* In the old (original Solaris 10) convention:
-               If there are more than 3 entries, there cannot be only the
-               ACE_OWNER, ACE_GROUP, ACE_OTHER entries.
-               In the newer Solaris 10 and Solaris 11 convention:
-               If there are more than 6 entries, there cannot be only the
-               ACE_OWNER, ACE_GROUP, ACE_EVERYONE entries, each once with
-               NEW_ACE_ACCESS_ALLOWED_ACE_TYPE and once with
-               NEW_ACE_ACCESS_DENIED_ACE_TYPE.  */
-            if (count > 6)
-              return 1;
-
-            entries = (ace_t *) malloc (count * sizeof (ace_t));
-            if (entries == NULL)
-              {
-                errno = ENOMEM;
-                return -1;
-              }
-            if (acl (name, ACE_GETACL, count, entries) == count)
-              {
-                if (acl_ace_nontrivial (count, entries))
-                  {
-                    free (entries);
-                    return 1;
-                  }
-                free (entries);
-                break;
-              }
-            /* Huh? The number of ACL entries changed since the last call.
-               Repeat.  */
-            free (entries);
-          }
-      }
+         file systems (whereas the other ones are used in UFS file systems).
+
+         In the old (original Solaris 10) convention:
+         If there are more than 3 entries, there cannot be only the
+         ACE_OWNER, ACE_GROUP, ACE_OTHER entries.
+         In the newer Solaris 10 and Solaris 11 convention:
+         If there are more than 6 entries, there cannot be only the
+         ACE_OWNER, ACE_GROUP, ACE_EVERYONE entries, each once with
+         NEW_ACE_ACCESS_ALLOWED_ACE_TYPE and once with
+         NEW_ACE_ACCESS_DENIED_ACE_TYPE.  */
+
+      ace_t entries[7];
+      int count = acl (name, ACE_GETACL, 7, entries);
+      if (0 <= count)
+        return acl_ace_nontrivial (count, entries);
+      if (! (errno == ENOSYS || errno == EINVAL))
+        return -1;
+
 #   endif

       return 0;
@@ -640,87 +567,48 @@ file_has_acl (char const *name, struct stat const *sb)

 # elif HAVE_GETACL /* HP-UX */

-      for (;;)
-        {
-          int count;
-          struct acl_entry entries[NACLENTRIES];
-
-          count = getacl (name, 0, NULL);
-
-          if (count < 0)
-            {
-              /* ENOSYS is seen on newer HP-UX versions.
-                 EOPNOTSUPP is typically seen on NFS mounts.
-                 ENOTSUP was seen on Quantum StorNext file systems (cvfs).  */
-              if (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTSUP)
-                break;
-              else
-                return -1;
-            }
-
-          if (count == 0)
-            return 0;
-
-          if (count > NACLENTRIES)
-            /* If NACLENTRIES cannot be trusted, use dynamic memory
-               allocation.  */
-            abort ();
+      struct acl_entry entries[4];
+      int count = getacl (name, 4, entries);

+      if (count < 0)
+        {
+          /* ENOSYS is seen on newer HP-UX versions.
+             EOPNOTSUPP is typically seen on NFS mounts.
+             ENOTSUP was seen on Quantum StorNext file systems (cvfs).  */
+          if (! (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTSUP))
+            return -1;
+        }
+      else if (count == 0)
+        return 0;
+      elif (count > 3)
+        {
           /* If there are more than 3 entries, there cannot be only the
              (uid,%), (%,gid), (%,%) entries.  */
-          if (count > 3)
-            return 1;
-
-          if (getacl (name, count, entries) == count)
-            {
-              struct stat statbuf;
-
-              if (stat (name, &statbuf) < 0)
-                return -1;
-
-              return acl_nontrivial (count, entries, &statbuf);
-            }
-          /* Huh? The number of ACL entries changed since the last call.
-             Repeat.  */
+          return 1;
         }
-
-#  if HAVE_ACLV_H /* HP-UX >= 11.11 */
-
-      for (;;)
+      else
         {
-          int count;
-          struct acl entries[NACLVENTRIES];
-
-          count = acl ((char *) name, ACL_CNT, NACLVENTRIES, entries);
+          struct stat statbuf;

-          if (count < 0)
-            {
-              /* EOPNOTSUPP is seen on NFS in HP-UX 11.11, 11.23.
-                 EINVAL is seen on NFS in HP-UX 11.31.  */
-              if (errno == ENOSYS || errno == EOPNOTSUPP || errno == EINVAL)
-                break;
-              else
-                return -1;
-            }
-
-          if (count == 0)
-            return 0;
+          if (stat (name, &statbuf) < 0)
+            return -1;

-          if (count > NACLVENTRIES)
-            /* If NACLVENTRIES cannot be trusted, use dynamic memory
-               allocation.  */
-            abort ();
+          return acl_nontrivial (count, entries, &statbuf);
+        }

-          /* If there are more than 4 entries, there cannot be only the
-             four base ACL entries.  */
-          if (count > 4)
-            return 1;
+#  if HAVE_ACLV_H /* HP-UX >= 11.11 */

-          if (acl ((char *) name, ACL_GET, count, entries) == count)
-            return aclv_nontrivial (count, entries);
-          /* Huh? The number of ACL entries changed since the last call.
-             Repeat.  */
-        }
+      {
+        struct acl entries[5];
+        int count = acl ((char *) name, ACL_GET, 5, entries);
+        if (0 <= count)
+          return aclv_nontrivial (count, entries);
+
+        /* EOPNOTSUPP is seen on NFS in HP-UX 11.11, 11.23.
+           EINVAL is seen on NFS in HP-UX 11.31.  */
+        if (! (errno == ENOSYS || errno == EOPNOTSUPP || errno == EINVAL))
+          return -1;
+      }

 #  endif

@@ -798,38 +686,12 @@ file_has_acl (char const *name, struct stat const *sb)
 # elif HAVE_ACLSORT /* NonStop Kernel */

       int count;
-      struct acl entries[NACLENTRIES];
-
-      for (;;)
-        {
-          count = acl ((char *) name, ACL_CNT, NACLENTRIES, NULL);
-
-          if (count < 0)
-            {
-              if (errno == ENOSYS || errno == ENOTSUP)
-                break;
-              else
-                return -1;
-            }
-
-          if (count == 0)
-            return 0;
-
-          if (count > NACLENTRIES)
-            /* If NACLENTRIES cannot be trusted, use dynamic memory
-               allocation.  */
-            abort ();
-
-          /* If there are more than 4 entries, there cannot be only the
-             four base ACL entries.  */
-          if (count > 4)
-            return 1;
-
-          if (acl ((char *) name, ACL_GET, count, entries) == count)
-            return acl_nontrivial (count, entries);
-          /* Huh? The number of ACL entries changed since the last call.
-             Repeat.  */
-        }
+      struct acl entries[5];
+      int count = acl ((char *) name, ACL_GET, 5, entries);
+      if (0 <= count)
+        return acl_nontrivial (count, entries);
+      if (! (errno == ENOSYS || errno == ENOTSUP))
+        return -1;

 # endif
     }
diff --git a/lib/set-mode-acl.c b/lib/set-mode-acl.c
index a81b321..7501818 100644
--- a/lib/set-mode-acl.c
+++ b/lib/set-mode-acl.c
@@ -213,49 +213,48 @@ qset_acl (char const *name, int desc, mode_t mode)
      In the new convention, these values are not used.  */
   int convention;

-  {
-    int count;
-    ace_t *entries;
-
-    for (;;)
-      {
-        if (desc != -1)
-          count = facl (desc, ACE_GETACLCNT, 0, NULL);
-        else
-          count = acl (name, ACE_GETACLCNT, 0, NULL);
-        if (count <= 0)
-          {
-            convention = -1;
-            break;
-          }
-        entries = (ace_t *) malloc (count * sizeof (ace_t));
-        if (entries == NULL)
-          {
-            errno = ENOMEM;
-            return -1;
-          }
-        if ((desc != -1
-             ? facl (desc, ACE_GETACL, count, entries)
-             : acl (name, ACE_GETACL, count, entries))
-            == count)
-          {
-            int i;
-
-            convention = 0;
-            for (i = 0; i < count; i++)
-              if (entries[i].a_flags & (OLD_ACE_OWNER | OLD_ACE_GROUP | OLD_ACE_OTHER))
-                {
-                  convention = 1;
-                  break;
-                }
-            free (entries);
-            break;
-          }
-        /* Huh? The number of ACL entries changed since the last call.
-           Repeat.  */
-        free (entries);
-      }
-  }
+  for (;;)
+    {
+      ace_t *entries;
+      int i, count1, acl_errno;
+      int count = (desc != -1
+                   ? facl (desc, ACE_GETACLCNT, 0, NULL)
+                   : acl (name, ACE_GETACLCNT, 0, NULL));
+      if (count <= 0)
+        {
+          convention = -1;
+          break;
+        }
+      if (! (count < MIN (INT_MAX, (size_t) -1 / sizeof (ace_t))
+             && (entries = (ace_t *) malloc ((count + 1) * sizeof (ace_t)))))
+        {
+          errno = ENOMEM;
+          return -1;
+        }
+      count1 = (desc != -1
+                ? facl (desc, ACE_GETACL, count + 1, entries)
+                : acl (name, ACE_GETACL, count + 1, entries));
+      acl_errno = errno;
+      if (count1 <= 0)
+        convention = -1;
+      else
+        {
+          convention = 0;
+          for (i = 0; i < count1; i++)
+            if (entries[i].a_flags
+                & (OLD_ACE_OWNER | OLD_ACE_GROUP | OLD_ACE_OTHER))
+              {
+                convention = 1;
+                break;
+              }
+        }
+      free (entries);
+      errno = acl_errno;
+      if (convention || count1 <= count)
+        break;
+      /* Huh? The number of ACL entries grew since the last call.
+         Repeat.  */
+    }

   if (convention >= 0)
     {