bug-gnulib
[Top][All Lists]
Advanced

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

Re: coreutils-8.12.178-df9cd on Solaris 10


From: Bruno Haible
Subject: Re: coreutils-8.12.178-df9cd on Solaris 10
Date: Mon, 5 Sep 2011 00:54:29 +0200
User-agent: KMail/1.13.6 (Linux/2.6.37.6-0.5-desktop; KDE/4.6.0; x86_64; ; )

Re <http://lists.gnu.org/archive/html/coreutils/2011-09/msg00064.html>:
> So, in summary, there are two problems:
>   - 'cp' creates a nontrivial ACL for a directory when the original
>     directory had none.
>   - chmod 700 does not change the 'delete_child' bit for owner to "allow".
> 
> I propose to fix the first problem. The second problem is IMO a bug in the
> kernel: /bin/chmod is affected as well.

This patch fixes it. I'm committing it in gnulib. It's possible that we
can further improve the robustness of the ACL handling by not using these
functions from Solaris libsec; we can see that later (after coreutils 8.13).

To understand what is a "trivial" ACL I used "chmod" on some file or
directory, followed by "/bin/ls -lvd". It turns out that with NFSv4 ACLs
the "trivial" ACLs consist of 6 entries: a "deny" and a "allow" entry for
each user / group / other.

And apparently, NFSv4 ACLs get broken when you apply an older style ACL
on the file. This is why the "done_setacl = 1;" statement below is essential.


2011-09-04  Bruno Haible  <address@hidden>

        acl: Improve support of NFSv4 ACLs on Solaris 10 (newer version).
        * lib/acl-internal.h (ACE_*, NEW_ACE_*): Define also on newer Solaris
        10.
        (OLD_ALLOW, OLD_DENY): New macros.
        (NEW_ACE_ACCESS_ALLOWED_ACE_TYPE): Renamed from
        ACE_ACCESS_ALLOWED_ACE_TYPE.
        (NEW_ACE_ACCESS_DENIED_ACE_TYPE): Renamed from
        ACE_ACCESS_DENIED_ACE_TYPE.
        (OLD_ACE_OWNER, OLD_ACE_GROUP, OLD_ACE_OTHER): New macros.
        (NEW_ACE_EXECUTE): Fix value.
        (NEW_ACE_APPEND_DATA, NEW_ACE_READ_NAMED_ATTRS,
        NEW_ACE_WRITE_NAMED_ATTRS, NEW_ACE_DELETE_CHILD,
        NEW_ACE_READ_ATTRIBUTES, NEW_ACE_WRITE_ATTRIBUTES, NEW_ACE_DELETE,
        NEW_ACE_READ_ACL, NEW_ACE_WRITE_ACL, NEW_ACE_WRITE_OWNER,
        NEW_ACE_SYNCHRONIZE): New macros.
        * lib/set-mode-acl.c (qset_acl): On newer Solaris 10, use acl or facl
        instead of acl_fromtext, acl_set, facl_set.
        Fixes a coreutils/tests/cp/perm failure.

--- lib/acl-internal.h.orig     Mon Sep  5 00:31:47 2011
+++ lib/acl-internal.h  Sun Sep  4 20:54:21 2011
@@ -189,7 +189,9 @@
    Return 0 if it is trivial, i.e. equivalent to a simple stat() mode.  */
 extern int acl_nontrivial (int count, aclent_t *entries);
 
-#   ifdef ACE_GETACL /* Solaris 10 */
+#  endif
+
+#  ifdef ACE_GETACL /* Solaris 10 */
 
 /* Test an ACL retrieved with ACE_GETACL.
    Return 1 if the given ACL, consisting of COUNT entries, is non-trivial.
@@ -199,19 +201,33 @@
 /* Definitions for when the built executable is executed on Solaris 10
    (newer version) or Solaris 11.  */
 /* For a_type.  */
-#    define ACE_ACCESS_ALLOWED_ACE_TYPE 0 /* replaces ALLOW */
-#    define ACE_ACCESS_DENIED_ACE_TYPE  1 /* replaces DENY */
+#   define OLD_ALLOW 0
+#   define OLD_DENY  1
+#   define NEW_ACE_ACCESS_ALLOWED_ACE_TYPE 0 /* replaces ALLOW */
+#   define NEW_ACE_ACCESS_DENIED_ACE_TYPE  1 /* replaces DENY */
 /* For a_flags.  */
-#    define NEW_ACE_OWNER            0x1000
-#    define NEW_ACE_GROUP            0x2000
-#    define NEW_ACE_IDENTIFIER_GROUP 0x0040
-#    define ACE_EVERYONE             0x4000
+#   define OLD_ACE_OWNER            0x0100
+#   define OLD_ACE_GROUP            0x0200
+#   define OLD_ACE_OTHER            0x0400
+#   define NEW_ACE_OWNER            0x1000
+#   define NEW_ACE_GROUP            0x2000
+#   define NEW_ACE_IDENTIFIER_GROUP 0x0040
+#   define NEW_ACE_EVERYONE         0x4000
 /* For a_access_mask.  */
-#    define NEW_ACE_READ_DATA  0x001 /* corresponds to 'r' */
-#    define NEW_ACE_WRITE_DATA 0x002 /* corresponds to 'w' */
-#    define NEW_ACE_EXECUTE    0x004 /* corresponds to 'x' */
-
-#   endif
+#   define NEW_ACE_READ_DATA         0x001 /* corresponds to 'r' */
+#   define NEW_ACE_WRITE_DATA        0x002 /* corresponds to 'w' */
+#   define NEW_ACE_APPEND_DATA       0x004
+#   define NEW_ACE_READ_NAMED_ATTRS  0x008
+#   define NEW_ACE_WRITE_NAMED_ATTRS 0x010
+#   define NEW_ACE_EXECUTE           0x020
+#   define NEW_ACE_DELETE_CHILD      0x040
+#   define NEW_ACE_READ_ATTRIBUTES   0x080
+#   define NEW_ACE_WRITE_ATTRIBUTES  0x100
+#   define NEW_ACE_DELETE          0x10000
+#   define NEW_ACE_READ_ACL        0x20000
+#   define NEW_ACE_WRITE_ACL       0x40000
+#   define NEW_ACE_WRITE_OWNER     0x80000
+#   define NEW_ACE_SYNCHRONIZE    0x100000
 
 #  endif
 
--- lib/set-mode-acl.c.orig     Mon Sep  5 00:31:47 2011
+++ lib/set-mode-acl.c  Sun Sep  4 23:46:00 2011
@@ -203,7 +203,7 @@
 
 # elif HAVE_FACL && defined GETACLCNT /* Solaris, Cygwin, not HP-UX */
 
-#  if defined ACL_NO_TRIVIAL
+#  if defined ACL_NO_TRIVIAL && 0
   /* Solaris 10 (newer version), which has additional API declared in
      <sys/acl.h> (acl_t) and implemented in libsec (acl_set, acl_trivial,
      acl_fromtext, ...).  */
@@ -250,6 +250,8 @@
 
 #  else /* Solaris, Cygwin, general case */
 
+  int done_setacl = 0;
+
 #   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).  */
@@ -292,7 +294,7 @@
 
             convention = 0;
             for (i = 0; i < count; i++)
-              if (entries[i].a_flags & (ACE_OWNER | ACE_GROUP | ACE_OTHER))
+              if (entries[i].a_flags & (OLD_ACE_OWNER | OLD_ACE_GROUP | 
OLD_ACE_OTHER))
                 {
                   convention = 1;
                   break;
@@ -308,90 +310,153 @@
 
   if (convention >= 0)
     {
-      ace_t entries[3];
+      ace_t entries[6];
+      int count;
       int ret;
 
       if (convention)
         {
           /* Running on Solaris 10.  */
-          entries[0].a_type = ALLOW;
-          entries[0].a_flags = ACE_OWNER;
+          entries[0].a_type = OLD_ALLOW;
+          entries[0].a_flags = OLD_ACE_OWNER;
           entries[0].a_who = 0; /* irrelevant */
           entries[0].a_access_mask = (mode >> 6) & 7;
-          entries[1].a_type = ALLOW;
-          entries[1].a_flags = ACE_GROUP;
+          entries[1].a_type = OLD_ALLOW;
+          entries[1].a_flags = OLD_ACE_GROUP;
           entries[1].a_who = 0; /* irrelevant */
           entries[1].a_access_mask = (mode >> 3) & 7;
-          entries[2].a_type = ALLOW;
-          entries[2].a_flags = ACE_OTHER;
+          entries[2].a_type = OLD_ALLOW;
+          entries[2].a_flags = OLD_ACE_OTHER;
           entries[2].a_who = 0;
           entries[2].a_access_mask = mode & 7;
+          count = 3;
         }
       else
         {
-          /* Running on Solaris 10 (newer version) or Solaris 11.  */
-          entries[0].a_type = ACE_ACCESS_ALLOWED_ACE_TYPE;
+          /* Running on Solaris 10 (newer version) or Solaris 11.
+             The details here were found through "/bin/ls -lvd somefiles".  */
+          struct stat statbuf;
+          int dir;
+
+          if ((desc != -1 ? fstat (desc, &statbuf) : stat (name, &statbuf)) >= 
0)
+            dir = S_ISDIR (statbuf.st_mode);
+          else
+            dir = 0;
+
+          entries[0].a_type = NEW_ACE_ACCESS_DENIED_ACE_TYPE;
           entries[0].a_flags = NEW_ACE_OWNER;
           entries[0].a_who = 0; /* irrelevant */
-          entries[0].a_access_mask =
-            (mode & 0400 ? NEW_ACE_READ_DATA : 0)
-            | (mode & 0200 ? NEW_ACE_WRITE_DATA : 0)
-            | (mode & 0100 ? NEW_ACE_EXECUTE : 0);
-          entries[1].a_type = ACE_ACCESS_ALLOWED_ACE_TYPE;
-          entries[1].a_flags = NEW_ACE_GROUP | NEW_ACE_IDENTIFIER_GROUP;
+          entries[0].a_access_mask = 0;
+          entries[1].a_type = NEW_ACE_ACCESS_ALLOWED_ACE_TYPE;
+          entries[1].a_flags = NEW_ACE_OWNER;
           entries[1].a_who = 0; /* irrelevant */
-          entries[1].a_access_mask =
-            (mode & 0040 ? NEW_ACE_READ_DATA : 0)
-            | (mode & 0020 ? NEW_ACE_WRITE_DATA : 0)
-            | (mode & 0010 ? NEW_ACE_EXECUTE : 0);
-          entries[2].a_type = ACE_ACCESS_ALLOWED_ACE_TYPE;
-          entries[2].a_flags = ACE_EVERYONE;
-          entries[2].a_who = 0;
-          entries[2].a_access_mask =
-            (mode & 0004 ? NEW_ACE_READ_DATA : 0)
-            | (mode & 0002 ? NEW_ACE_WRITE_DATA : 0)
-            | (mode & 0001 ? NEW_ACE_EXECUTE : 0);
+          entries[1].a_access_mask = NEW_ACE_WRITE_NAMED_ATTRS
+                                     | NEW_ACE_WRITE_ATTRIBUTES
+                                     | NEW_ACE_WRITE_ACL
+                                     | NEW_ACE_WRITE_OWNER;
+          if (mode & 0400)
+            entries[1].a_access_mask |= NEW_ACE_READ_DATA;
+          else
+            entries[0].a_access_mask |= NEW_ACE_READ_DATA;
+          if (mode & 0200)
+            entries[1].a_access_mask |= NEW_ACE_WRITE_DATA | 
NEW_ACE_APPEND_DATA;
+          else
+            entries[0].a_access_mask |= NEW_ACE_WRITE_DATA | 
NEW_ACE_APPEND_DATA;
+          if (mode & 0100)
+            entries[1].a_access_mask |= NEW_ACE_EXECUTE;
+          else
+            entries[0].a_access_mask |= NEW_ACE_EXECUTE;
+          entries[2].a_type = NEW_ACE_ACCESS_DENIED_ACE_TYPE;
+          entries[2].a_flags = NEW_ACE_GROUP | NEW_ACE_IDENTIFIER_GROUP;
+          entries[2].a_who = 0; /* irrelevant */
+          entries[2].a_access_mask = 0;
+          entries[3].a_type = NEW_ACE_ACCESS_ALLOWED_ACE_TYPE;
+          entries[3].a_flags = NEW_ACE_GROUP | NEW_ACE_IDENTIFIER_GROUP;
+          entries[3].a_who = 0; /* irrelevant */
+          entries[3].a_access_mask = 0;
+          if (mode & 0040)
+            entries[3].a_access_mask |= NEW_ACE_READ_DATA;
+          else
+            entries[2].a_access_mask |= NEW_ACE_READ_DATA;
+          if (mode & 0020)
+            entries[3].a_access_mask |= NEW_ACE_WRITE_DATA | 
NEW_ACE_APPEND_DATA;
+          else
+            entries[2].a_access_mask |= NEW_ACE_WRITE_DATA | 
NEW_ACE_APPEND_DATA;
+          if (mode & 0010)
+            entries[3].a_access_mask |= NEW_ACE_EXECUTE;
+          else
+            entries[2].a_access_mask |= NEW_ACE_EXECUTE;
+          entries[4].a_type = NEW_ACE_ACCESS_DENIED_ACE_TYPE;
+          entries[4].a_flags = ACE_EVERYONE;
+          entries[4].a_who = 0;
+          entries[4].a_access_mask = NEW_ACE_WRITE_NAMED_ATTRS
+                                     | NEW_ACE_WRITE_ATTRIBUTES
+                                     | NEW_ACE_WRITE_ACL
+                                     | NEW_ACE_WRITE_OWNER;
+          entries[5].a_type = NEW_ACE_ACCESS_ALLOWED_ACE_TYPE;
+          entries[5].a_flags = ACE_EVERYONE;
+          entries[5].a_who = 0;
+          entries[5].a_access_mask = NEW_ACE_READ_NAMED_ATTRS
+                                     | NEW_ACE_READ_ATTRIBUTES
+                                     | NEW_ACE_READ_ACL
+                                     | NEW_ACE_SYNCHRONIZE;
+          if (mode & 0004)
+            entries[5].a_access_mask |= NEW_ACE_READ_DATA;
+          else
+            entries[4].a_access_mask |= NEW_ACE_READ_DATA;
+          if (mode & 0002)
+            entries[5].a_access_mask |= NEW_ACE_WRITE_DATA | 
NEW_ACE_APPEND_DATA;
+          else
+            entries[4].a_access_mask |= NEW_ACE_WRITE_DATA | 
NEW_ACE_APPEND_DATA;
+          if (mode & 0001)
+            entries[5].a_access_mask |= NEW_ACE_EXECUTE;
+          else
+            entries[4].a_access_mask |= NEW_ACE_EXECUTE;
+          count = 6;
         }
       if (desc != -1)
-        ret = facl (desc, ACE_SETACL,
-                    sizeof (entries) / sizeof (ace_t), entries);
+        ret = facl (desc, ACE_SETACL, count, entries);
       else
-        ret = acl (name, ACE_SETACL,
-                   sizeof (entries) / sizeof (ace_t), entries);
+        ret = acl (name, ACE_SETACL, count, entries);
       if (ret < 0 && errno != EINVAL && errno != ENOTSUP)
         {
           if (errno == ENOSYS)
             return chmod_or_fchmod (name, desc, mode);
           return -1;
         }
+      if (ret == 0)
+        done_setacl = 1;
     }
 #   endif
 
-  {
-    aclent_t entries[3];
-    int ret;
+  if (!done_setacl)
+    {
+      aclent_t entries[3];
+      int ret;
 
-    entries[0].a_type = USER_OBJ;
-    entries[0].a_id = 0; /* irrelevant */
-    entries[0].a_perm = (mode >> 6) & 7;
-    entries[1].a_type = GROUP_OBJ;
-    entries[1].a_id = 0; /* irrelevant */
-    entries[1].a_perm = (mode >> 3) & 7;
-    entries[2].a_type = OTHER_OBJ;
-    entries[2].a_id = 0;
-    entries[2].a_perm = mode & 7;
-
-    if (desc != -1)
-      ret = facl (desc, SETACL, sizeof (entries) / sizeof (aclent_t), entries);
-    else
-      ret = acl (name, SETACL, sizeof (entries) / sizeof (aclent_t), entries);
-    if (ret < 0)
-      {
-        if (errno == ENOSYS || errno == EOPNOTSUPP)
-          return chmod_or_fchmod (name, desc, mode);
-        return -1;
-      }
-  }
+      entries[0].a_type = USER_OBJ;
+      entries[0].a_id = 0; /* irrelevant */
+      entries[0].a_perm = (mode >> 6) & 7;
+      entries[1].a_type = GROUP_OBJ;
+      entries[1].a_id = 0; /* irrelevant */
+      entries[1].a_perm = (mode >> 3) & 7;
+      entries[2].a_type = OTHER_OBJ;
+      entries[2].a_id = 0;
+      entries[2].a_perm = mode & 7;
+
+      if (desc != -1)
+        ret = facl (desc, SETACL,
+                    sizeof (entries) / sizeof (aclent_t), entries);
+      else
+        ret = acl (name, SETACL,
+                   sizeof (entries) / sizeof (aclent_t), entries);
+      if (ret < 0)
+        {
+          if (errno == ENOSYS || errno == EOPNOTSUPP)
+            return chmod_or_fchmod (name, desc, mode);
+          return -1;
+        }
+    }
 
   if (!MODE_INSIDE_ACL || (mode & (S_ISUID | S_ISGID | S_ISVTX)))
     {
-- 
In memoriam Erich Fellgiebel <http://en.wikipedia.org/wiki/Erich_Fellgiebel>



reply via email to

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