Date: Wed, 7 Mar 2018 13:47:01 +0000 (UTC) From: Andriy Gapon <avg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-vendor@freebsd.org Subject: svn commit: r330591 - vendor-sys/illumos/dist/uts/common/fs/zfs Message-ID: <201803071347.w27Dl17t022789@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 <szaydel@racktopsystems.com> Reviewed by: Paul B. Henson <henson@acm.org> Reviewed by: Prakash Surya <prakash.surya@delphix.com> Approved by: Matthew Ahrens <mahrens@delphix.com> Author: Dominik Hassler <hadfl@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) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201803071347.w27Dl17t022789>