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.

Signed-off-by: Ameer Hamza <ahamza@ixsystems.com>

include/os/freebsd/zfs/sys/zfs_dir.h

@@ -61,7 +61,8 @@ extern void zfs_unlinked_add(znode_t *, dmu_tx_t *);
 extern void zfs_unlinked_drain(zfsvfs_t *zfsvfs);
 extern int zfs_sticky_remove_access(znode_t *, znode_t *, cred_t *cr);
 extern int zfs_get_xattrdir(znode_t *, znode_t **, cred_t *, int);
-extern int zfs_make_xattrdir(znode_t *, vattr_t *, znode_t **, cred_t *);
+extern int zfs_make_xattrdir(znode_t *, vattr_t *, znode_t **, cred_t *,
+    boolean_t);
 
 #ifdef	__cplusplus
 }

include/os/linux/zfs/sys/zfs_dir.h

@@ -68,7 +68,8 @@ extern void zfs_unlinked_drain(zfsvfs_t *zfsvfs);
 extern void zfs_unlinked_drain_stop_wait(zfsvfs_t *zfsvfs);
 extern int zfs_sticky_remove_access(znode_t *, znode_t *, cred_t *cr);
 extern int zfs_get_xattrdir(znode_t *, znode_t **, cred_t *, int);
-extern int zfs_make_xattrdir(znode_t *, vattr_t *, znode_t **, cred_t *);
+extern int zfs_make_xattrdir(znode_t *, vattr_t *, znode_t **, cred_t *,
+    boolean_t);
 
 #ifdef	__cplusplus
 }

module/os/freebsd/zfs/zfs_dir.c

@@ -796,7 +796,8 @@ zfs_dirempty(znode_t *dzp)
 }
 
 int
-zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xvpp, cred_t *cr)
+zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xvpp, cred_t *cr,
+    boolean_t skip_acl)
 {
 	zfsvfs_t *zfsvfs = zp->z_zfsvfs;
 	znode_t *xzp;
@@ -806,6 +807,7 @@ zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xvpp, cred_t *cr)
 	boolean_t fuid_dirtied;
 	uint64_t parent __maybe_unused;
 
+	(void) skip_acl;
 	*xvpp = NULL;
 
 	if ((error = zfs_acl_ids_create(zp, IS_XATTR, vap, cr, NULL,
@@ -911,7 +913,7 @@ zfs_get_xattrdir(znode_t *zp, znode_t **xzpp, cred_t *cr, int flags)
 	va.va_mode = S_IFDIR | S_ISVTX | 0777;
 	zfs_fuid_map_ids(zp, cr, &va.va_uid, &va.va_gid);
 
-	error = zfs_make_xattrdir(zp, &va, xzpp, cr);
+	error = zfs_make_xattrdir(zp, &va, xzpp, cr, B_TRUE);
 
 	if (error == ERESTART) {
 		/* NB: we already did dmu_tx_wait() if necessary */

module/os/linux/zfs/zfs_dir.c

@@ -1098,7 +1098,8 @@ zfs_dirempty(znode_t *dzp)
 }
 
 int
-zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xzpp, cred_t *cr)
+zfs_make_xattrdir(znode_t *zp, vattr_t *vap, znode_t **xzpp, cred_t *cr,
+    boolean_t skip_acl)
 {
 	zfsvfs_t *zfsvfs = ZTOZSB(zp);
 	znode_t *xzp;
@@ -1112,7 +1113,7 @@ 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,
+	if ((error = zfs_zaccess(zp, ACE_WRITE_NAMED_ATTRS, 0, skip_acl, cr,
 	    kcred->user_ns)))
 		return (error);
 
@@ -1185,6 +1186,7 @@ zfs_get_xattrdir(znode_t *zp, znode_t **xzpp, cred_t *cr, int flags)
 	zfs_dirlock_t	*dl;
 	vattr_t		va;
 	int		error;
+	boolean_t	skip_acl = (flags & ATTR_NOACLCHECK) ? B_TRUE : B_FALSE;
 top:
 	error = zfs_dirent_lock(&dl, zp, "", &xzp, ZXATTR, NULL, NULL);
 	if (error)
@@ -1221,7 +1223,7 @@ zfs_get_xattrdir(znode_t *zp, znode_t **xzpp, cred_t *cr, int flags)
 	zfs_fuid_map_ids(zp, cr, &va.va_uid, &va.va_gid);
 
 	va.va_dentry = NULL;
-	error = zfs_make_xattrdir(zp, &va, xzpp, cr);
+	error = zfs_make_xattrdir(zp, &va, xzpp, cr, skip_acl);
 	zfs_dirent_unlock(dl);
 
 	if (error == ERESTART) {

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);

module/os/linux/zfs/zpl_xattr.c

@@ -470,8 +470,8 @@ zpl_xattr_set_dir(struct inode *ip, const char *name, const void *value,
 	if (value != NULL)
 		lookup_flags |= CREATE_XATTR_DIR;
 
-	error = -zfs_lookup(ITOZ(ip), NULL, &dxzp, lookup_flags,
-	    cr, NULL, NULL);
+	error = -zfs_lookup(ITOZ(ip), NULL, &dxzp, lookup_flags |
+	    ATTR_NOACLCHECK, cr, NULL, NULL);
 	if (error)
 		goto out;
 
@@ -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;
 	}

module/zfs/zfs_replay.c

@@ -570,7 +570,8 @@ zfs_replay_create(void *arg1, void *arg2, boolean_t byteswap)
 
 		break;
 	case TX_MKXATTR:
-		error = zfs_make_xattrdir(dzp, &xva.xva_vattr, &zp, kcred);
+		error = zfs_make_xattrdir(dzp, &xva.xva_vattr, &zp, kcred,
+		    B_TRUE);
 		break;
 	case TX_SYMLINK:
 		name = (char *)(lr + 1);

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"