From e3785718bac2ee21664ad6a2b7b503f82f533425 Mon Sep 17 00:00:00 2001 From: Ameer Hamza <106930537+ixhamza@users.noreply.github.com> Date: Mon, 12 Dec 2022 23:21:37 +0500 Subject: [PATCH] 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 Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Reviewed-by: Youzhong Yang Signed-off-by: Ameer Hamza Closes #14220 --- module/os/linux/zfs/zfs_dir.c | 4 ---- module/os/linux/zfs/zfs_vnops_os.c | 3 ++- module/os/linux/zfs/zpl_xattr.c | 2 +- tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh | 3 +++ 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/module/os/linux/zfs/zfs_dir.c b/module/os/linux/zfs/zfs_dir.c index 85aa94d8d..09eac37f9 100644 --- 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 index 94ae5e91f..a94af0ea3 100644 --- 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 @@ top: * 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 index 99d9b3793..b9e74bcbb 100644 --- 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 index 7906f5063..ffb5b4db7 100755 --- 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"