[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
>
