Re: [PATCH 1/6] ntfs3: rework xattr handlers and switch to POSIX ACL VFS helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[Sorry, forgot to Cc ntfs3 developers.]

On Mon, Aug 29, 2022 at 02:38:40PM +0200, Christian Brauner wrote:
> The xattr code in ntfs3 is currently a bit confused. For example, it
> defines a POSIX ACL i_op->set_acl() method but instead of relying on the
> generic POSIX ACL VFS helpers it defines its own set of xattr helpers
> with the consequence that i_op->set_acl() is currently dead code.
> 
> Switch ntfs3 to rely on the VFS POSIX ACL xattr handlers. Also remove
> i_op->{g,s}et_acl() methods from symlink inode operations. Symlinks
> don't support xattrs.
> 
> This is a preliminary change for the following patches which move
> handling idmapped mounts directly in posix_acl_xattr_set().
> 
> This survives POSIX ACL xfstests.
> 
> Fixes: be71b5cba2e6 ("fs/ntfs3: Add attrib operations")
> Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>
> ---
>  fs/ntfs3/inode.c |   2 -
>  fs/ntfs3/xattr.c | 102 +++--------------------------------------------
>  2 files changed, 6 insertions(+), 98 deletions(-)
> 
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index 51363d4e8636..26a76ebfe58f 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -1927,8 +1927,6 @@ const struct inode_operations ntfs_link_inode_operations = {
>  	.setattr	= ntfs3_setattr,
>  	.listxattr	= ntfs_listxattr,
>  	.permission	= ntfs_permission,
> -	.get_acl	= ntfs_get_acl,
> -	.set_acl	= ntfs_set_acl,
>  };
>  
>  const struct address_space_operations ntfs_aops = {
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 6ae1f56b7358..7de8718c68a9 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -625,67 +625,6 @@ int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
>  	return ntfs_set_acl_ex(mnt_userns, inode, acl, type, false);
>  }
>  
> -static int ntfs_xattr_get_acl(struct user_namespace *mnt_userns,
> -			      struct inode *inode, int type, void *buffer,
> -			      size_t size)
> -{
> -	struct posix_acl *acl;
> -	int err;
> -
> -	if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
> -		ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
> -		return -EOPNOTSUPP;
> -	}
> -
> -	acl = ntfs_get_acl(inode, type, false);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -
> -	if (!acl)
> -		return -ENODATA;
> -
> -	err = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> -	posix_acl_release(acl);
> -
> -	return err;
> -}
> -
> -static int ntfs_xattr_set_acl(struct user_namespace *mnt_userns,
> -			      struct inode *inode, int type, const void *value,
> -			      size_t size)
> -{
> -	struct posix_acl *acl;
> -	int err;
> -
> -	if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
> -		ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
> -		return -EOPNOTSUPP;
> -	}
> -
> -	if (!inode_owner_or_capable(mnt_userns, inode))
> -		return -EPERM;
> -
> -	if (!value) {
> -		acl = NULL;
> -	} else {
> -		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -		if (IS_ERR(acl))
> -			return PTR_ERR(acl);
> -
> -		if (acl) {
> -			err = posix_acl_valid(&init_user_ns, acl);
> -			if (err)
> -				goto release_and_out;
> -		}
> -	}
> -
> -	err = ntfs_set_acl(mnt_userns, inode, acl, type);
> -
> -release_and_out:
> -	posix_acl_release(acl);
> -	return err;
> -}
> -
>  /*
>   * ntfs_init_acl - Initialize the ACLs of a new inode.
>   *
> @@ -852,23 +791,6 @@ static int ntfs_getxattr(const struct xattr_handler *handler, struct dentry *de,
>  		goto out;
>  	}
>  
> -#ifdef CONFIG_NTFS3_FS_POSIX_ACL
> -	if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
> -		     sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
> -	    (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
> -		     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
> -		/* TODO: init_user_ns? */
> -		err = ntfs_xattr_get_acl(
> -			&init_user_ns, inode,
> -			name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
> -				? ACL_TYPE_ACCESS
> -				: ACL_TYPE_DEFAULT,
> -			buffer, size);
> -		goto out;
> -	}
> -#endif
>  	/* Deal with NTFS extended attribute. */
>  	err = ntfs_get_ea(inode, name, name_len, buffer, size, NULL);
>  
> @@ -981,22 +903,6 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
>  		goto out;
>  	}
>  
> -#ifdef CONFIG_NTFS3_FS_POSIX_ACL
> -	if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
> -		     sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
> -	    (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
> -		     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
> -		err = ntfs_xattr_set_acl(
> -			mnt_userns, inode,
> -			name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
> -				? ACL_TYPE_ACCESS
> -				: ACL_TYPE_DEFAULT,
> -			value, size);
> -		goto out;
> -	}
> -#endif
>  	/* Deal with NTFS extended attribute. */
>  	err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
>  
> @@ -1086,7 +992,7 @@ static bool ntfs_xattr_user_list(struct dentry *dentry)
>  }
>  
>  // clang-format off
> -static const struct xattr_handler ntfs_xattr_handler = {
> +static const struct xattr_handler ntfs_other_xattr_handler = {
>  	.prefix	= "",
>  	.get	= ntfs_getxattr,
>  	.set	= ntfs_setxattr,
> @@ -1094,7 +1000,11 @@ static const struct xattr_handler ntfs_xattr_handler = {
>  };
>  
>  const struct xattr_handler *ntfs_xattr_handlers[] = {
> -	&ntfs_xattr_handler,
> +#ifdef CONFIG_NTFS3_FS_POSIX_ACL
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
> +#endif
> +	&ntfs_other_xattr_handler,
>  	NULL,
>  };
>  // clang-format on
> -- 
> 2.34.1
> 



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux