Author: avg
Date: Wed Mar  7 13:47:01 2018
New Revision: 330591
URL: https://svnweb.freebsd.org/changeset/base/330591

Log:
  8984 fix for 6764 breaks ACL inheritance
  
  illumos/illumos-gate@e9bacc6d1a71ea3f7082038b2868de8c4dd98bdc
  
https://github.com/illumos/illumos-gate/commit/e9bacc6d1a71ea3f7082038b2868de8c4dd98bdc
  
  https://www.illumos.org/issues/8984
    Consider a directory configured as:
    drwx-ws---+ 2 henson cpp 3 Jan 23 12:35 dropbox/
    user:henson:rwxpdDaARWcC--:f-i----:allow
    owner@:--------------:f-i----:allow
    group@:--------------:f-i----:allow
    everyone@:--------------:f-i----:allow
    owner@:rwxpdDaARWcC--:-di----:allow
    group:cpp:-wx-----------:-------:allow
    owner@:rwxpdDaARWcC--:-------:allow
    A new file created in this directory ends up looking like:
    rw-r--r-+ 1 astudent cpp 0 Jan 23 12:39 testfile
    user:henson:rw-pdDaARWcC--:------I:allow
    owner@:--------------:------I:allow
    group@:--------------:------I:allow
    everyone@:--------------:------I:allow
    owner@:rw-p--aARWcCos:-------:allow
    group@:r-----a-R-c--s:-------:allow
    everyone@:r-----a-R-c--s:-------:allow
    with extraneous group@ and everyone@ entries allowing read access that
    shouldn't exist.
    Per Albert Lee on the zfs mailing list:
    "aclinherit=passthrough/passthrough-x should still
    ignore the requested mode when an inheritable ACE for owner@ group@,
    or everyone@ is present in the parent directory.
    It appears there was an oversight in my fix for
    https://www.illumos.org/issues/6764 which made calling zfs_acl_chmod
    from zfs_acl_inherit unconditional. I think the parent ACL check for
    aclinherit=passthrough needs to be reintroduced in zfs_acl_inherit."
    We have a large number of faculty who use dropbox directories like the 
example
    to have students submit projects. All of these directories are now allowing
  
  Reviewed by: Sam Zaydel <szay...@racktopsystems.com>
  Reviewed by: Paul B. Henson <hen...@acm.org>
  Reviewed by: Prakash Surya <prakash.su...@delphix.com>
  Approved by: Matthew Ahrens <mahr...@delphix.com>
  Author: Dominik Hassler <ha...@omniosce.org>

Modified:
  vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c

Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c Wed Mar  7 13:45:29 
2018        (r330590)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c Wed Mar  7 13:47:01 
2018        (r330591)
@@ -1494,7 +1494,7 @@ zfs_ace_can_use(vtype_t vtype, uint16_t acep_flags)
  */
 static zfs_acl_t *
 zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_t vtype, zfs_acl_t *paclp,
-    uint64_t mode)
+    uint64_t mode, boolean_t *need_chmod)
 {
        void            *pacep = NULL;
        void            *acep;
@@ -1508,7 +1508,10 @@ zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_t vtype, zfs_a
        size_t          data1sz, data2sz;
        uint_t          aclinherit;
        boolean_t       isdir = (vtype == VDIR);
+       boolean_t       isreg = (vtype == VREG);
 
+       *need_chmod = B_TRUE;
+
        aclp = zfs_acl_alloc(paclp->z_version);
        aclinherit = zfsvfs->z_acl_inherit;
        if (aclinherit == ZFS_ACL_DISCARD || vtype == VLNK)
@@ -1531,6 +1534,17 @@ zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_t vtype, zfs_a
                        continue;
 
                /*
+                * If owner@, group@, or everyone@ inheritable
+                * then zfs_acl_chmod() isn't needed.
+                */
+               if ((aclinherit == ZFS_ACL_PASSTHROUGH ||
+                   aclinherit == ZFS_ACL_PASSTHROUGH_X) &&
+                   ((iflags & (ACE_OWNER|ACE_EVERYONE)) ||
+                   ((iflags & OWNING_GROUP) == OWNING_GROUP)) &&
+                   (isreg || (isdir && (iflags & ACE_DIRECTORY_INHERIT_ACE))))
+                       *need_chmod = B_FALSE;
+
+               /*
                 * Strip inherited execute permission from file if
                 * not in mode
                 */
@@ -1617,6 +1631,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *va
        zfsvfs_t        *zfsvfs = dzp->z_zfsvfs;
        zfs_acl_t       *paclp;
        gid_t           gid;
+       boolean_t       need_chmod = B_TRUE;
        boolean_t       trim = B_FALSE;
        boolean_t       inherited = B_FALSE;
 
@@ -1706,7 +1721,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *va
                        VERIFY(0 == zfs_acl_node_read(dzp, B_TRUE,
                            &paclp, B_FALSE));
                        acl_ids->z_aclp = zfs_acl_inherit(zfsvfs,
-                           vap->va_type, paclp, acl_ids->z_mode);
+                           vap->va_type, paclp, acl_ids->z_mode, &need_chmod);
                        inherited = B_TRUE;
                } else {
                        acl_ids->z_aclp =
@@ -1716,15 +1731,18 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *va
                mutex_exit(&dzp->z_lock);
                mutex_exit(&dzp->z_acl_lock);
 
-               if (vap->va_type == VDIR)
-                       acl_ids->z_aclp->z_hints |= ZFS_ACL_AUTO_INHERIT;
+               if (need_chmod) {
+                       if (vap->va_type == VDIR)
+                               acl_ids->z_aclp->z_hints |=
+                                   ZFS_ACL_AUTO_INHERIT;
 
-               if (zfsvfs->z_acl_mode == ZFS_ACL_GROUPMASK &&
-                   zfsvfs->z_acl_inherit != ZFS_ACL_PASSTHROUGH &&
-                   zfsvfs->z_acl_inherit != ZFS_ACL_PASSTHROUGH_X)
-                       trim = B_TRUE;
-               zfs_acl_chmod(vap->va_type, acl_ids->z_mode, B_FALSE, trim,
-                   acl_ids->z_aclp);
+                       if (zfsvfs->z_acl_mode == ZFS_ACL_GROUPMASK &&
+                           zfsvfs->z_acl_inherit != ZFS_ACL_PASSTHROUGH &&
+                           zfsvfs->z_acl_inherit != ZFS_ACL_PASSTHROUGH_X)
+                               trim = B_TRUE;
+                       zfs_acl_chmod(vap->va_type, acl_ids->z_mode, B_FALSE,
+                           trim, acl_ids->z_aclp);
+               }
        }
 
        if (inherited || vsecp) {
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to