Author: mav
Date: Wed May 11 13:50:34 2016
New Revision: 299452
URL: https://svnweb.freebsd.org/changeset/base/299452
Log:
MFV r299451: 6764 zfs issues with inheritance flags during chmod(2) with
aclmode=passthrough
Reviewed by: Gordon Ross <g...@nexenta.com>
Reviewed by: Yuri Pankov <yuri.pan...@nexenta.com>
Author: Albert Lee <tr...@nexenta.com>
openzfs/openzfs@1bcf0d240bdebed61b4261f7c0ee323e07c8dfac
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 May
11 13:49:50 2016 (r299451)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Wed May
11 13:50:34 2016 (r299452)
@@ -884,7 +884,6 @@ zfs_set_ace(zfs_acl_t *aclp, void *acep,
/*
* Determine mode of file based on ACL.
- * Also, create FUIDs for any User/Group ACEs
*/
uint64_t
zfs_mode_compute(uint64_t fmode, zfs_acl_t *aclp,
@@ -910,11 +909,9 @@ zfs_mode_compute(uint64_t fmode, zfs_acl
entry_type = (iflags & ACE_TYPE_FLAGS);
/*
- * Skip over owner@, group@ or everyone@ inherit only ACEs
+ * Skip over any inherit_only ACEs
*/
- if ((iflags & ACE_INHERIT_ONLY_ACE) &&
- (entry_type == ACE_OWNER || entry_type == ACE_EVERYONE ||
- entry_type == OWNING_GROUP))
+ if (iflags & ACE_INHERIT_ONLY_ACE)
continue;
if (entry_type == ACE_OWNER || (entry_type == 0 &&
@@ -1330,7 +1327,8 @@ zfs_aclset_common(znode_t *zp, zfs_acl_t
}
static void
-zfs_acl_chmod(vtype_t vtype, uint64_t mode, boolean_t trim, zfs_acl_t *aclp)
+zfs_acl_chmod(vtype_t vtype, uint64_t mode, boolean_t split, boolean_t trim,
+ zfs_acl_t *aclp)
{
void *acep = NULL;
uint64_t who;
@@ -1375,15 +1373,27 @@ zfs_acl_chmod(vtype_t vtype, uint64_t mo
while (acep = zfs_acl_next_ace(aclp, acep, &who, &access_mask,
&iflags, &type)) {
- uint16_t inherit_flags;
-
entry_type = (iflags & ACE_TYPE_FLAGS);
- inherit_flags = (iflags & ALL_INHERIT);
-
- if ((entry_type == ACE_OWNER || entry_type == ACE_EVERYONE ||
- (entry_type == OWNING_GROUP)) &&
- ((inherit_flags & ACE_INHERIT_ONLY_ACE) == 0)) {
- continue;
+ /*
+ * ACEs used to represent the file mode may be divided
+ * into an equivalent pair of inherit-only and regular
+ * ACEs, if they are inheritable.
+ * Skip regular ACEs, which are replaced by the new mode.
+ */
+ if (split && (entry_type == ACE_OWNER ||
+ entry_type == OWNING_GROUP ||
+ entry_type == ACE_EVERYONE)) {
+ if (!isdir || !(iflags &
+ (ACE_FILE_INHERIT_ACE|ACE_DIRECTORY_INHERIT_ACE)))
+ continue;
+ /*
+ * We preserve owner@, group@, or @everyone
+ * permissions, if they are inheritable, by
+ * copying them to inherit_only ACEs. This
+ * prevents inheritable permissions from being
+ * altered along with the file mode.
+ */
+ iflags |= ACE_INHERIT_ONLY_ACE;
}
/*
@@ -1391,12 +1401,12 @@ zfs_acl_chmod(vtype_t vtype, uint64_t mo
* the hints (which are later masked into the pflags)
* so create knows to do inheritance.
*/
- if (isdir && (inherit_flags &
+ if (isdir && (iflags &
(ACE_FILE_INHERIT_ACE|ACE_DIRECTORY_INHERIT_ACE)))
aclp->z_hints |= ZFS_INHERIT_ACE;
if ((type != ALLOW && type != DENY) ||
- (inherit_flags & ACE_INHERIT_ONLY_ACE)) {
+ (iflags & ACE_INHERIT_ONLY_ACE)) {
switch (type) {
case ACE_ACCESS_ALLOWED_OBJECT_ACE_TYPE:
case ACE_ACCESS_DENIED_OBJECT_ACE_TYPE:
@@ -1406,7 +1416,6 @@ zfs_acl_chmod(vtype_t vtype, uint64_t mo
break;
}
} else {
-
/*
* Limit permissions granted by ACEs to be no greater
* than permissions of the requested group mode.
@@ -1422,11 +1431,11 @@ zfs_acl_chmod(vtype_t vtype, uint64_t mo
new_count++;
new_bytes += ace_size;
}
- zfs_set_ace(aclp, zacep, masks.owner, 0, -1, ACE_OWNER);
+ zfs_set_ace(aclp, zacep, masks.owner, ALLOW, -1, ACE_OWNER);
zacep = (void *)((uintptr_t)zacep + abstract_size);
- zfs_set_ace(aclp, zacep, masks.group, 0, -1, OWNING_GROUP);
+ zfs_set_ace(aclp, zacep, masks.group, ALLOW, -1, OWNING_GROUP);
zacep = (void *)((uintptr_t)zacep + abstract_size);
- zfs_set_ace(aclp, zacep, masks.everyone, 0, -1, ACE_EVERYONE);
+ zfs_set_ace(aclp, zacep, masks.everyone, ALLOW, -1, ACE_EVERYONE);
new_count += 3;
new_bytes += abstract_size * 3;
@@ -1452,7 +1461,7 @@ zfs_acl_chmod_setattr(znode_t *zp, zfs_a
if (error == 0) {
(*aclp)->z_hints = zp->z_pflags & V4_ACL_WIDE_FLAGS;
- zfs_acl_chmod(ZTOV(zp)->v_type, mode,
+ zfs_acl_chmod(ZTOV(zp)->v_type, mode, B_TRUE,
(zp->z_zfsvfs->z_acl_mode == ZFS_ACL_GROUPMASK), *aclp);
}
mutex_exit(&zp->z_lock);
@@ -1462,21 +1471,6 @@ zfs_acl_chmod_setattr(znode_t *zp, zfs_a
}
/*
- * strip off write_owner and write_acl
- */
-static void
-zfs_restricted_update(zfsvfs_t *zfsvfs, zfs_acl_t *aclp, void *acep)
-{
- uint32_t mask = aclp->z_ops.ace_mask_get(acep);
-
- if ((zfsvfs->z_acl_inherit == ZFS_ACL_RESTRICTED) &&
- (aclp->z_ops.ace_type_get(acep) == ALLOW)) {
- mask &= ~RESTRICTED_CLEAR;
- aclp->z_ops.ace_mask_set(acep, mask);
- }
-}
-
-/*
* Should ACE be inherited?
*/
static int
@@ -1497,9 +1491,9 @@ zfs_ace_can_use(vtype_t vtype, uint16_t
*/
static zfs_acl_t *
zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_t vtype, zfs_acl_t *paclp,
- uint64_t mode, boolean_t *need_chmod)
+ uint64_t mode)
{
- void *pacep;
+ void *pacep = NULL;
void *acep;
zfs_acl_node_t *aclnode;
zfs_acl_t *aclp = NULL;
@@ -1509,22 +1503,14 @@ zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_
size_t ace_size;
void *data1, *data2;
size_t data1sz, data2sz;
- boolean_t vdir = vtype == VDIR;
- boolean_t vreg = vtype == VREG;
- boolean_t passthrough, passthrough_x, noallow;
-
- passthrough_x =
- zfsvfs->z_acl_inherit == ZFS_ACL_PASSTHROUGH_X;
- passthrough = passthrough_x ||
- zfsvfs->z_acl_inherit == ZFS_ACL_PASSTHROUGH;
- noallow =
- zfsvfs->z_acl_inherit == ZFS_ACL_NOALLOW;
+ uint_t aclinherit;
+ boolean_t isdir = (vtype == VDIR);
- *need_chmod = B_TRUE;
- pacep = NULL;
aclp = zfs_acl_alloc(paclp->z_version);
- if (zfsvfs->z_acl_inherit == ZFS_ACL_DISCARD || vtype == VLNK)
+ aclinherit = zfsvfs->z_acl_inherit;
+ if (aclinherit == ZFS_ACL_DISCARD || vtype == VLNK)
return (aclp);
+
while (pacep = zfs_acl_next_ace(paclp, pacep, &who,
&access_mask, &iflags, &type)) {
@@ -1534,31 +1520,31 @@ zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_
if (!zfs_acl_valid_ace_type(type, iflags))
continue;
- if (noallow && type == ALLOW)
- continue;
-
- ace_size = aclp->z_ops.ace_size(pacep);
-
- if (!zfs_ace_can_use(vtype, iflags))
+ /*
+ * Check if ACE is inheritable by this vnode
+ */
+ if ((aclinherit == ZFS_ACL_NOALLOW && type == ALLOW) ||
+ !zfs_ace_can_use(vtype, iflags))
continue;
/*
- * If owner@, group@, or everyone@ inheritable
- * then zfs_acl_chmod() isn't needed.
+ * Strip inherited execute permission from file if
+ * not in mode
*/
- if (passthrough &&
- ((iflags & (ACE_OWNER|ACE_EVERYONE)) ||
- ((iflags & OWNING_GROUP) ==
- OWNING_GROUP)) && (vreg || (vdir && (iflags &
- ACE_DIRECTORY_INHERIT_ACE)))) {
- *need_chmod = B_FALSE;
+ if (aclinherit == ZFS_ACL_PASSTHROUGH_X && type == ALLOW &&
+ !isdir && ((mode & (S_IXUSR|S_IXGRP|S_IXOTH)) == 0)) {
+ access_mask &= ~ACE_EXECUTE;
}
- if (!vdir && passthrough_x &&
- ((mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)) {
- access_mask &= ~ACE_EXECUTE;
+ /*
+ * Strip write_acl and write_owner from permissions
+ * when inheriting an ACE
+ */
+ if (aclinherit == ZFS_ACL_RESTRICTED && type == ALLOW) {
+ access_mask &= ~RESTRICTED_CLEAR;
}
+ ace_size = aclp->z_ops.ace_size(pacep);
aclnode = zfs_acl_node_alloc(ace_size);
list_insert_tail(&aclp->z_acl, aclnode);
acep = aclnode->z_acldata;
@@ -1580,18 +1566,21 @@ zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_
aclp->z_acl_bytes += aclnode->z_size;
newflags = aclp->z_ops.ace_flags_get(acep);
- if (vdir)
- aclp->z_hints |= ZFS_INHERIT_ACE;
-
- if ((iflags & ACE_NO_PROPAGATE_INHERIT_ACE) || !vdir) {
+ /*
+ * If ACE is not to be inherited further, or if the vnode is
+ * not a directory, remove all inheritance flags
+ */
+ if (!isdir || (iflags & ACE_NO_PROPAGATE_INHERIT_ACE)) {
newflags &= ~ALL_INHERIT;
aclp->z_ops.ace_flags_set(acep,
newflags|ACE_INHERITED_ACE);
- zfs_restricted_update(zfsvfs, aclp, acep);
continue;
}
- ASSERT(vdir);
+ /*
+ * This directory has an inheritable ACE
+ */
+ aclp->z_hints |= ZFS_INHERIT_ACE;
/*
* If only FILE_INHERIT is set then turn on
@@ -1608,12 +1597,14 @@ zfs_acl_inherit(zfsvfs_t *zfsvfs, vtype_
newflags|ACE_INHERITED_ACE);
}
}
+
return (aclp);
}
/*
* Create file system object initial permissions
* including inheritable ACEs.
+ * Also, create FUIDs for owner and group.
*/
int
zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
@@ -1623,7 +1614,7 @@ zfs_acl_ids_create(znode_t *dzp, int fla
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;
bzero(acl_ids, sizeof (zfs_acl_ids_t));
@@ -1716,7 +1707,7 @@ zfs_acl_ids_create(znode_t *dzp, int fla
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, &need_chmod);
+ vap->va_type, paclp, acl_ids->z_mode);
inherited = B_TRUE;
} else {
acl_ids->z_aclp =
@@ -1725,13 +1716,16 @@ zfs_acl_ids_create(znode_t *dzp, int fla
}
mutex_exit(&dzp->z_lock);
mutex_exit(&dzp->z_acl_lock);
- if (need_chmod) {
- acl_ids->z_aclp->z_hints |= (vap->va_type == VDIR) ?
- ZFS_ACL_AUTO_INHERIT : 0;
- zfs_acl_chmod(vap->va_type, acl_ids->z_mode,
- (zfsvfs->z_acl_mode == ZFS_ACL_GROUPMASK),
- acl_ids->z_aclp);
- }
+
+ 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 (inherited || vsecp) {
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
