From owner-svn-src-all@freebsd.org Wed Mar 7 13:49:27 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 82ED9F2DCC6; Wed, 7 Mar 2018 13:49:27 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3422074604; Wed, 7 Mar 2018 13:49:27 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 2DE9922215; Wed, 7 Mar 2018 13:49:27 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w27DnRg0022930; Wed, 7 Mar 2018 13:49:27 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w27DnRfY022929; Wed, 7 Mar 2018 13:49:27 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201803071349.w27DnRfY022929@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Wed, 7 Mar 2018 13:49:27 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r330592 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Group: head X-SVN-Commit-Author: avg X-SVN-Commit-Paths: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Commit-Revision: 330592 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Mar 2018 13:49:27 -0000 Author: avg Date: Wed Mar 7 13:49:26 2018 New Revision: 330592 URL: https://svnweb.freebsd.org/changeset/base/330592 Log: MFV r330591: 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 Reviewed by: Paul B. Henson Reviewed by: Prakash Surya Approved by: Matthew Ahrens Author: Dominik Hassler PR: 216886 MFC after: 2 weeks Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Directory Properties: head/sys/cddl/contrib/opensolaris/ (props changed) Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Wed Mar 7 13:47:01 2018 (r330591) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Wed Mar 7 13:49:26 2018 (r330592) @@ -1476,7 +1476,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; @@ -1490,7 +1490,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) @@ -1513,6 +1516,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 */ @@ -1599,6 +1613,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; @@ -1694,7 +1709,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *va !(dzp->z_pflags & ZFS_XATTR)) { VERIFY(0 == zfs_acl_node_read(dzp, &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 = @@ -1703,15 +1718,18 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *va } 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) {