bug-gnulib
[Top][All Lists]
Advanced

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

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


From: Paul Eggert
Subject: Re: bug#10639: BUG REPORT coreutils-8.15 Solaris 10 64bit
Date: Wed, 01 Feb 2012 23:07:22 -0800
User-agent: Mozilla/5.0 (X11; Linux i686; rv:9.0) Gecko/20111229 Thunderbird/9.0

On 02/01/2012 02:22 AM, Bruno Haible wrote:
> I find it overkill to change the code for HP-UX and NonStop systems
> when the report is about Solaris. Also I think the structure of the loop
> is not the problem; it is the code's reaction to
>   acl("x", ACE_GETACL, 4, 0x3432A16E0)            Err#-1

The patch you sent looks like it'll fix that particular bug, but while
looking into this I discovered so many bugs in this area, mostly
hard-to-test race conditions and overflows, that I thought it better
to rewrite the affected code than to try to fix each bug one at a time.
I didn't write this up very well, and my first cut at doing this could
stand some improvements of its own.  Here's a revised patch that tries
to do a better job at all this.  (If it's any consolation, this patch
makes the code simpler -- 186 lines shorter....)


diff --git a/ChangeLog b/ChangeLog
index f80c7dd..ec16bef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,89 @@
+2012-02-01  Paul Eggert  <address@hidden>
+
+       acl: fix several problems with ACLs
+       Problem with infinite loop on Solaris 10 + vxfs reported by Bill
+       Jones in <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10639>.
+       Looking into the code, I saw several closely related issues:
+
+        - There's a race condition in this kind of code:
+
+            n = acl (f, GETACLCNT, 0, NULL);
+            [ allocate an array A of size N ]
+            if (acl (f, GETACL, n, a) == n)
+              return ok;
+
+          The ACL can grow in size between the first and second calls to
+          'acl', which means that the second 'acl' returns a truncated
+          ACL but thinks that it has the whole thing.  To avoid this, the
+          following pattern should be used instead:
+
+            n = acl (f, GETACLCNT, 0, NULL);
+            [ allocate an array A of size N + 1 ]
+            n1 = acl (f, GETACL, n + 1, a);
+            if (0 < n1 && n1 <= n)
+              return ok;
+
+        - There were several instances of this pattern:
+
+            for (;;) {
+              n = acl (f, GETACLCNT, 0, NULL);
+              [ allocate an array A of size N ]
+              if (acl (f, GETACL, n, a) == n)
+                break;
+            }
+
+          This loop might never terminate if some other process is constantly
+          manipulating the file's ACL.  The loop should be rewritten to
+          terminate.
+
+        - The acl (... GETACLNT ...) call is merely an optimization; its value
+          is merely a hint as to how big to make the array.  A better
+          optimization is to avoid the acl (... GETACLNT ...)  call entirely,
+          and just guess a reasonably-big size, growing the size and trying
+          again if it's not large enough.  This guarantees termination, and
+          saves a system call.
+
+        - With this approach, for ports like HP-UX that have an upper bound
+          for the ACL length, there's no longer any need to loop reading the
+          ACL.  Just read it once and you're done reading it.
+
+        - For ports like Solaris that do need to loop (because the ACL length
+          is not known a priori), it's faster to allocate a reasonably-sized
+          buffer on the stack and use that, and to allocate something on the
+          heap only if the ACL is unusually large.  This avoids malloc in the
+          usual case.
+
+        - The code that calculated sizes of these arrays did not check for
+          overflow in size calculations; it should.
+
+        - There are some memory leaks.  For example, in qcopy_acl, if acl
+          (src_name, GETACLCNT, 0, NULL) < 0 && errno == EPERM, then the
+          storage for ace_entries leaks.  Similarly, if acl (src_name,
+          ACE_GETACL, ace_count, ace_entries) returns -1, the storage for
+          ace_entries leaks.
+
+        - In qset_acl, there's sometimes no need to read the entire ACL
+          to determine the convention; this can save system calls when
+          looking at very large ACLs.
+
+       Rather than fix these bugs one at a time I thought it more efficient
+       to rewrite the affected code, as follows:
+       * lib/acl-internal.h (GETACLCNT): Remove; no longer needed.
+       Include <limits.h>.
+       (MIN): New macro.
+       * lib/copy-acl.c (qcopy_acl): Don't bother to count ACL entries
+       before getting them.  Instead, just get them, and make sure that
+       the number of entries gotten is less than the number requested.
+       This avoids a race condition and omits a system call in the usual
+       case.  Before, it could have been the case that we asked for and
+       received N entries, but these were the first N of more than N
+       actual entries, if the ACL was modified while we were getting it.
+       With this change, there is no need to use GETACLCNT or similar ops.
+       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.
+
 2012-01-31  Karl Berry  <address@hidden>
 
        setstate doc: typo.
diff --git a/lib/acl-internal.h b/lib/acl-internal.h
index 88e5e45..1548ce1 100644
--- a/lib/acl-internal.h
+++ b/lib/acl-internal.h
@@ -26,9 +26,6 @@
 #if HAVE_SYS_ACL_H
 # include <sys/acl.h>
 #endif
-#if defined HAVE_FACL && ! defined GETACLCNT && defined ACL_CNT
-# define GETACLCNT ACL_CNT
-#endif
 
 /* On Linux, additional ACL related API is available in <acl/libacl.h>.  */
 #ifdef HAVE_ACL_LIBACL_H
@@ -55,6 +52,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..2c24b36 100644
--- a/lib/copy-acl.c
+++ b/lib/copy-acl.c
@@ -181,11 +181,25 @@ 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
+  enum {
+    ace_alloc_init = 4000 / sizeof (ace_t),
+    ace_alloc_max = MIN (INT_MAX, (size_t) -1 / sizeof (ace_t))
+  };
   int ace_count;
-  ace_t *ace_entries;
+  int ace_alloc = ace_alloc_init;
+  ace_t ace_buf[ace_alloc_init];
+  ace_t *ace_entries = ace_buf;
 # endif
+  void *ace_malloced = NULL;
+  enum {
+    acl_alloc_init = 4000 / sizeof (aclent_t),
+    acl_alloc_max = MIN (INT_MAX, (size_t) -1 / sizeof (aclent_t))
+  };
   int count;
-  aclent_t *entries;
+  int acl_alloc = acl_alloc_init;
+  aclent_t acl_buf[acl_alloc_init];
+  aclent_t *entries = acl_buf;
+  aclent_t *acl_malloced = NULL;
   int did_chmod;
   int saved_errno;
   int ret;
@@ -204,85 +218,71 @@ qcopy_acl (const char *src_name, int source_desc, const 
char *dst_name,
      that the kernel will translate the ACL from one form to the other.
      (See in <http://docs.sun.com/app/docs/doc/819-2241/6n4huc7ia?l=en&a=view>
      the description of ENOTSUP.)  */
-  for (;;)
+  while ((ace_count = (source_desc != -1
+                       ? facl (source_desc, ACE_GETACL, ace_alloc, ace_entries)
+                       : acl (src_name, ACE_GETACL, ace_alloc, ace_entries)))
+         == ace_alloc)
     {
-      ace_count = (source_desc != -1
-                   ? facl (source_desc, ACE_GETACLCNT, 0, NULL)
-                   : acl (src_name, ACE_GETACLCNT, 0, NULL));
-
-      if (ace_count < 0)
-        {
-          if (errno == ENOSYS || errno == EINVAL)
-            {
-              ace_count = 0;
-              ace_entries = NULL;
-              break;
-            }
-          else
-            return -2;
-        }
-
-      if (ace_count == 0)
-        {
-          ace_entries = NULL;
-          break;
-        }
-
-      ace_entries = (ace_t *) malloc (ace_count * sizeof (ace_t));
-      if (ace_entries == NULL)
+      free (ace_malloced);
+      ace_alloc = (ace_alloc <= ace_alloc_max / 2
+                   ? 2 * ace_alloc
+                   : ace_alloc_max);
+      ace_entries = ace_malloced = (ace_count == ace_alloc_max
+                                    ? NULL
+                                    : malloc (ace_alloc * sizeof (ace_t)));
+      if (! ace_malloced)
         {
           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)
-        break;
-      /* Huh? The number of ACL entries changed since the last call.
-         Repeat.  */
     }
-# endif
 
-  for (;;)
+  if (ace_count < 0)
     {
-      count = (source_desc != -1
-               ? facl (source_desc, GETACLCNT, 0, NULL)
-               : acl (src_name, GETACLCNT, 0, NULL));
-
-      if (count < 0)
+      int ace_errno = errno;
+      if (! (ace_errno == ENOSYS || ace_errno == EINVAL))
         {
-          if (errno == ENOSYS || errno == ENOTSUP || errno == EOPNOTSUPP)
-            {
-              count = 0;
-              entries = NULL;
-              break;
-            }
-          else
-            return -2;
+          free (ace_malloced);
+          errno = ace_errno;
+          return -2;
         }
+      ace_count = 0;
+    }
 
-      if (count == 0)
-        {
-          entries = NULL;
-          break;
-        }
+# endif
 
-      entries = (aclent_t *) malloc (count * sizeof (aclent_t));
-      if (entries == NULL)
+  while ((count = (source_desc != -1
+                   ? facl (source_desc, GETACL, acl_alloc, entries)
+                   : acl (src_name, GETACL, acl_alloc, entries)))
+         == acl_alloc)
+    {
+      free (acl_malloced);
+      acl_alloc = (acl_alloc <= acl_alloc_max / 2
+                   ? 2 * acl_alloc
+                   : acl_alloc_max);
+      entries = acl_malloced = (count == acl_alloc_max
+                                ? NULL
+                                : malloc (acl_alloc * sizeof (aclent_t)));
+      if (! acl_malloced)
         {
+          free (ace_malloced);
           errno = ENOMEM;
           return -2;
         }
+    }
 
-      if ((source_desc != -1
-           ? facl (source_desc, GETACL, count, entries)
-           : acl (src_name, GETACL, count, entries))
-          == count)
-        break;
-      /* Huh? The number of ACL entries changed since the last call.
-         Repeat.  */
+  if (count < 0)
+    {
+      int acl_errno = errno;
+      if (! (acl_errno == ENOSYS || acl_errno == ENOTSUP
+             || acl_errno == EOPNOTSUPP))
+        {
+          free (ace_malloced);
+          free (acl_malloced);
+          errno = acl_errno;
+          return -2;
+        }
+      count = 0;
     }
 
   /* Is there an ACL of either kind?  */
@@ -290,7 +290,11 @@ qcopy_acl (const char *src_name, int source_desc, const 
char *dst_name,
   if (ace_count == 0)
 # endif
     if (count == 0)
-      return qset_acl (dst_name, dest_desc, mode);
+      {
+        free (ace_malloced);
+        free (acl_malloced);
+        return qset_acl (dst_name, dest_desc, mode);
+      }
 
   did_chmod = 0; /* set to 1 once the mode bits in 0777 have been set */
   saved_errno = 0; /* the first non-ignorable error code */
@@ -324,7 +328,7 @@ qcopy_acl (const char *src_name, int source_desc, const 
char *dst_name,
       else
         did_chmod = 1;
     }
-  free (entries);
+  free (acl_malloced);
 
 # ifdef ACE_GETACL
   if (ace_count > 0)
@@ -340,7 +344,7 @@ qcopy_acl (const char *src_name, int source_desc, const 
char *dst_name,
             saved_errno = 0;
         }
     }
-  free (ace_entries);
+  free (ace_malloced);
 # endif
 
   if (MODE_INSIDE_ACL
@@ -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));
+  count = (source_desc != -1
+           ? fgetacl (source_desc, NACLENTRIES + 1, entries)
+           : getacl (src_name, NACLENTRIES + 1, entries));
 
-      if (count < 0)
-        {
-          if (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTSUP)
-            {
-              count = 0;
-              break;
-            }
-          else
-            return -2;
-        }
-
-      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 (NACLENTRIES < count)
+    /* 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..41e750f 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -518,7 +518,7 @@ file_has_acl (char const *name, struct stat const *sb)
         return ACL_NOT_WELL_SUPPORTED (errno) ? 0 : -1;
       return ret;
 
-# elif HAVE_FACL && defined GETACLCNT /* Solaris, Cygwin, not HP-UX */
+# elif HAVE_FACL && defined GETACL /* Solaris, Cygwin, not HP-UX */
 
 #  if defined ACL_NO_TRIVIAL
 
@@ -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..a1df9be 100644
--- a/lib/set-mode-acl.c
+++ b/lib/set-mode-acl.c
@@ -197,7 +197,7 @@ qset_acl (char const *name, int desc, mode_t mode)
   return chmod_or_fchmod (name, desc, mode);
 #  endif
 
-# elif HAVE_FACL && defined GETACLCNT /* Solaris, Cygwin, not HP-UX */
+# elif HAVE_FACL && defined GETACL /* Solaris, Cygwin, not HP-UX */
 
   int done_setacl = 0;
 
@@ -214,30 +214,23 @@ qset_acl (char const *name, int desc, mode_t mode)
   int convention;
 
   {
-    int count;
-    ace_t *entries;
+    enum {
+      alloc_init = 4000 / sizeof (ace_t),
+      alloc_max = MIN (INT_MAX, (size_t) -1 / sizeof (ace_t))
+    };
+    int alloc = alloc_init;
+    ace_t buf[alloc_init];
+    ace_t *entries = buf;
+    ace_t *malloced = NULL;
 
     for (;;)
       {
-        if (desc != -1)
-          count = facl (desc, ACE_GETACLCNT, 0, NULL);
-        else
-          count = acl (name, ACE_GETACLCNT, 0, NULL);
+        int count = (desc != -1
+                     ? facl (desc, ACE_GETACL, alloc, entries)
+                     : acl (name, ACE_GETACL, alloc, entries));
         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)
+          convention = -1;
+        else
           {
             int i;
 
@@ -248,12 +241,18 @@ qset_acl (char const *name, int desc, mode_t mode)
                   convention = 1;
                   break;
                 }
-            free (entries);
-            break;
           }
-        /* Huh? The number of ACL entries changed since the last call.
-           Repeat.  */
-        free (entries);
+        free (malloced);
+        if (count < alloc || convention != 0)
+          break;
+        alloc = alloc <= alloc_max / 2 ? 2 * alloc : alloc_max;
+        entries = malloced =
+          count == alloc_max ? NULL : malloc (alloc * sizeof *entries);
+        if (! malloced)
+          {
+            errno = ENOMEM;
+            return -1;
+          }
       }
   }
 



reply via email to

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