On Thu, Jan 02, 2020 at 05:28:44PM -0500, Olga Kornievskaia wrote:
> Hi Andreas and Bruce,
>
> I thought of you folks as somebody who might have a strong opinion on
> the topic. Right now an NFS client is limited to setting and getting
> ACLs that can't be larger than 64K (XATTR_SIZE_MAX) (where some NFS
> server don't have such limit on the ACL size). There are limits in
> fs/xattr.c during getting and setting xattrs. I believe that's because
> linux local xattr system is limited to that particular constraint.
> However, an NFS acl that uses the xattr interface can be larger than
> that. Is it at all possible to suggest to the larger FS community to
> remove those limits or would that be a non-starter?
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 90dd78f..52a3f91 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -428,8 +428,6 @@ int __vfs_setxattr_noperm(struct dentry *dentry,
> const char *name,
> return error;
>
> if (size) {
> - if (size > XATTR_SIZE_MAX)
> - return -E2BIG;
> kvalue = kvmalloc(size, GFP_KERNEL);
> if (!kvalue)
> return -ENOMEM;
> @@ -528,8 +526,6 @@ static int path_setxattr(const char __user *pathname,
> return error;
>
> if (size) {
> - if (size > XATTR_SIZE_MAX)
> - size = XATTR_SIZE_MAX;
> kvalue = kvzalloc(size, GFP_KERNEL);
> if (!kvalue)
> return -ENOMEM;
Aside from not wanting to allocate a raw amount of kernel memory based
on a system call parameter without any checks, I support the idea :-)
The internal xattr interface can be a little awkward to deal with,
the static upper limit being one of the issues that caused me some
problems when implementing user xattrs for NFS.
In general, I would love to see paged-based xattr kernel interfaces,
treating extended attributes as a secondary data stream, which would
make caching fit in a lot more naturally, and means all FS-specific
caching implementations could be removed in favor of a generic one.
One issue right now is that, an xattr not being a 'stream', a lot
of FS code caches the entire value in kmalloc-ed space, which becomes
unwieldy if the XATTR_SIZE_MAX limit is removed.
In other words, I think removing the limit won't work that well with
the current implementation, but I hope that the implementation can
be changed so that the limit can be removed.
- Frank
