Skip permission checks for extended attributes

zfs_zaccess_trivial() calls the generic_permission() to read xattr attributes. This causes deadlock if called from zpl_xattr_set_dir() context as xattr and the dent locks are already held in this scenario. This commit skips the permissions checks for extended attributes since the Linux VFS stack already checks it before passing us the control. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Youzhong Yang <yyang@mathworks.com> Signed-off-by: Ameer Hamza <ahamza@ixsystems.com> Closes openzfs#14220
truenas · Dec 12, 2022 · e378571 · e378571
1 parent f900279
commit e378571
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 6 deletions.
diff --git a/module/os/linux/zfs/zfs_dir.c b/module/os/linux/zfs/zfs_dir.c
@@ -1112,10 +1112,6 @@ zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xzpp, cred_t *cr)
 
 	*xzpp = NULL;
 
-	if ((error = zfs_zaccess(zp, ACE_WRITE_NAMED_ATTRS, 0, B_FALSE, cr,
-	    kcred->user_ns)))
-		return (error);
-
 	if ((error = zfs_acl_ids_create(zp, IS_XATTR, vap, cr, NULL,
 	    &acl_ids, kcred->user_ns)) != 0)
 		return (error);

diff --git a/module/os/linux/zfs/zfs_vnops_os.c b/module/os/linux/zfs/zfs_vnops_os.c
@@ -555,6 +555,7 @@ zfs_create(znode_t *dzp, char *name, vattr_t *vap, int excl,
 	boolean_t	fuid_dirtied;
 	boolean_t	have_acl = B_FALSE;
 	boolean_t	waited = B_FALSE;
+	boolean_t	skip_acl = (flag & ATTR_NOACLCHECK) ? B_TRUE : B_FALSE;
 
 	/*
 	 * If we have an ephemeral id, ACL, or XVATTR then
@@ -627,7 +628,7 @@ zfs_create(znode_t *dzp, char *name, vattr_t *vap, int excl,
 		 * Create a new file object and update the directory
 		 * to reference it.
 		 */
-		if ((error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, B_FALSE, cr,
+		if ((error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, skip_acl, cr,
 		    mnt_ns))) {
 			if (have_acl)
 				zfs_acl_ids_free(&acl_ids);

diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c
@@ -499,7 +499,7 @@ zpl_xattr_set_dir(struct inode *ip, const char *name, const void *value,
 		vap->va_gid = crgetgid(cr);
 
 		error = -zfs_create(dxzp, (char *)name, vap, 0, 0644, &xzp,
-		    cr, 0, NULL, kcred->user_ns);
+		    cr, ATTR_NOACLCHECK, NULL, kcred->user_ns);
 		if (error)
 			goto out;
 	}

diff --git a/tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh b/tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh
@@ -35,6 +35,7 @@
 # STRATEGY:
 #	1. Prepare an appropriate ACL on the test directory
 #	2. Change the owner of the directory
+#	3. Reset and set the ACLs for test directory owned by the user
 #
 
 verify_runnable "both"
@@ -44,6 +45,8 @@ log_must setfacl -d -m u:$ZFS_ACL_STAFF1:rwx $TESTDIR
 log_must setfacl -b $TESTDIR
 
 log_must chown $ZFS_ACL_STAFF1 $TESTDIR
+log_must setfacl -b $TESTDIR
+log_must setfacl -d -m u:$ZFS_ACL_STAFF1:rwx $TESTDIR
 log_must chown 0 $TESTDIR
 
 log_pass "chown works with POSIX ACLs"