lkml.org 
[lkml]   [2021]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 2/2] afs: Stop listxattr() from listing "afs.*" attributes
Date
Am 12.03.21 um 00:06 schrieb David Howells:
> afs_listxattr() lists all the available special afs xattrs (i.e. those in
> the "afs.*" space), no matter what type of server we're dealing with. But
> OpenAFS servers, for example, cannot deal with some of the extra-capable
> attributes that AuriStor (YFS) servers provide. Unfortunately, the
> presence of the afs.yfs.* attributes causes errors[1] for anything that
> tries to read them if the server is of the wrong type.
>
> Fix the problem by removing afs_listxattr() so that none of the special
> xattrs are listed (AFS doesn't support xattrs). It does mean, however,
> that getfattr won't list them, though they can still be accessed with
> getxattr() and setxattr().
>
> This can be tested with something like:
>
> getfattr -d -m ".*" /afs/example.com/path/to/file
>
> With this change, none of the afs.* attributes should be visible.
>
> Changes:
> ver #2:
> - Hide all of the afs.* xattrs, not just the ACL ones.
>
> Fixes: ae46578b963f ("afs: Get YFS ACLs and information through xattrs")
> Reported-by: Gaja Sophie Peters <gaja.peters@math.uni-hamburg.de>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-afs@lists.infradead.org
> Link: http://lists.infradead.org/pipermail/linux-afs/2021-March/003502.html [1]
> Link: http://lists.infradead.org/pipermail/linux-afs/2021-March/003567.html # v1
> ---
>
> fs/afs/dir.c | 1 -
> fs/afs/file.c | 1 -
> fs/afs/inode.c | 1 -
> fs/afs/internal.h | 1 -
> fs/afs/mntpt.c | 1 -
> fs/afs/xattr.c | 23 -----------------------
> 6 files changed, 28 deletions(-)
>
> diff --git a/fs/afs/dir.c b/fs/afs/dir.c
> index 714fcca9af99..17548c1faf02 100644
> --- a/fs/afs/dir.c
> +++ b/fs/afs/dir.c
> @@ -70,7 +70,6 @@ const struct inode_operations afs_dir_inode_operations = {
> .permission = afs_permission,
> .getattr = afs_getattr,
> .setattr = afs_setattr,
> - .listxattr = afs_listxattr,
> };
>
> const struct address_space_operations afs_dir_aops = {
> diff --git a/fs/afs/file.c b/fs/afs/file.c
> index 85f5adf21aa0..960b64268623 100644
> --- a/fs/afs/file.c
> +++ b/fs/afs/file.c
> @@ -43,7 +43,6 @@ const struct inode_operations afs_file_inode_operations = {
> .getattr = afs_getattr,
> .setattr = afs_setattr,
> .permission = afs_permission,
> - .listxattr = afs_listxattr,
> };
>
> const struct address_space_operations afs_fs_aops = {
> diff --git a/fs/afs/inode.c b/fs/afs/inode.c
> index 1156b2df28d3..12be88716e4c 100644
> --- a/fs/afs/inode.c
> +++ b/fs/afs/inode.c
> @@ -27,7 +27,6 @@
>
> static const struct inode_operations afs_symlink_inode_operations = {
> .get_link = page_get_link,
> - .listxattr = afs_listxattr,
> };
>
> static noinline void dump_vnode(struct afs_vnode *vnode, struct afs_vnode *parent_vnode)
> diff --git a/fs/afs/internal.h b/fs/afs/internal.h
> index b626e38e9ab5..1627b1872812 100644
> --- a/fs/afs/internal.h
> +++ b/fs/afs/internal.h
> @@ -1509,7 +1509,6 @@ extern int afs_launder_page(struct page *);
> * xattr.c
> */
> extern const struct xattr_handler *afs_xattr_handlers[];
> -extern ssize_t afs_listxattr(struct dentry *, char *, size_t);
>
> /*
> * yfsclient.c
> diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
> index 052dab2f5c03..bbb2c210d139 100644
> --- a/fs/afs/mntpt.c
> +++ b/fs/afs/mntpt.c
> @@ -32,7 +32,6 @@ const struct inode_operations afs_mntpt_inode_operations = {
> .lookup = afs_mntpt_lookup,
> .readlink = page_readlink,
> .getattr = afs_getattr,
> - .listxattr = afs_listxattr,
> };
>
> const struct inode_operations afs_autocell_inode_operations = {
> diff --git a/fs/afs/xattr.c b/fs/afs/xattr.c
> index 4934e325a14a..7751b0b3f81d 100644
> --- a/fs/afs/xattr.c
> +++ b/fs/afs/xattr.c
> @@ -11,29 +11,6 @@
> #include <linux/xattr.h>
> #include "internal.h"
>
> -static const char afs_xattr_list[] =
> - "afs.acl\0"
> - "afs.cell\0"
> - "afs.fid\0"
> - "afs.volume\0"
> - "afs.yfs.acl\0"
> - "afs.yfs.acl_inherited\0"
> - "afs.yfs.acl_num_cleaned\0"
> - "afs.yfs.vol_acl";
> -
> -/*
> - * Retrieve a list of the supported xattrs.
> - */
> -ssize_t afs_listxattr(struct dentry *dentry, char *buffer, size_t size)
> -{
> - if (size == 0)
> - return sizeof(afs_xattr_list);
> - if (size < sizeof(afs_xattr_list))
> - return -ERANGE;
> - memcpy(buffer, afs_xattr_list, sizeof(afs_xattr_list));
> - return sizeof(afs_xattr_list);
> -}
> -
> /*
> * Deal with the result of a successful fetch ACL operation.
> */

Tested-by: Gaja Sophie Peters <gaja.peters@math.uni-hamburg.de>

Works for me:
$ getfattr -d -m - /afs/openafs.org/
$ getfattr -d -m - /afs/your-file-system.com/
these two show nothing as expected

$ getfattr -n afs.acl /afs/openafs.org/
$ getfattr -n afs.acl /afs/your-file-system.com/
these two show the ACL as expected

$ getfattr -n afs.yfs.acl /afs/openafs.org/
$ getfattr -n afs.yfs.acl /afs/your-file-system.com/
the latter shows as expected the YFS-ACL,
the former as expected the simple message "No such attribute".


Only as a "BTW", the primary Patch-Description has a minor mistake: In
the email titled "[PATCH v2 0/2] AFS metadata xattr fixes", you write:

> Fix an oops in AFS that can be triggered by accessing one of the
> afs.yfs.* xattrs against a yfs server[1][2]

That should of course be "against an OpenAFS server".

Greetings,
Gaja Peters

--
+----------
| IT-Gruppe, Systemadministration
| Universität Hamburg, Fachbereich Mathematik
| Bundesstr. 55 (Geomatikum)
| Raum 212; Tel. 42838-5175
+----------

\
 
 \ /
  Last update: 2021-03-12 22:01    [W:0.270 / U:0.988 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site