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 #14220

diff --git a/module/os/linux/zfs/zfs_dir.c b/module/os/linux/zfs/zfs_dir.c
index 85aa94d8df6a3e0272fb7e4d6436425c7bc48dce..09eac37f9e74e8bc232f6bf08dc3bce6bdc5aeb3 100644 (file)
--- 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 94ae5e91f1c4f42af6437e4e9406d68c4ab0861a..a94af0ea3bd3fbcb1b0503761c8700d28a834c4a 100644 (file)
--- 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 99d9b3793f297d1a5803b4d98268f535deabcb89..b9e74bcbb4c983fb946065a084e19faeb41c703f 100644 (file)
--- 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 7906f5063c810a46f3663be2114c5cc0f02c1d27..ffb5b4db71b48aa703b49ed6931ccd052d00d381 100755 (executable)
--- 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"