Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAN-5tyEg2b1CnbJrc-Hf2406cPWCAOjYcpPq0FremYjFXsytDQ@mail.gmail.com>
 <20200102233109.GA8735@dev-dsk-fllinden-2c-c1893d73.us-west-2.amazon.com> <CAN-5tyH9-FeNsa-Vc+v7wRui=bqEqwaqscDBx+6gnE90SSmZFQ@mail.gmail.com>
In-Reply-To: <CAN-5tyH9-FeNsa-Vc+v7wRui=bqEqwaqscDBx+6gnE90SSmZFQ@mail.gmail.com>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Fri, 3 Jan 2020 12:18:15 -0500
Message-ID: <CAN-5tyGydXiJjLKJAHOMGj_z_UmMT_MdSrTgSiBu45aEYEZauw@mail.gmail.com>
Subject: Re: extended attributes limitation of xattr_size_max
To:     Frank van der Linden <fllinden@amazon.com>
Cc:     Andreas Gruenbacher <agruen@kernel.org>,
        "J. Bruce Fields" <bfields@redhat.com>,
        linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 3, 2020 at 11:13 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Thu, Jan 2, 2020 at 6:31 PM Frank van der Linden <fllinden@amazon.com> wrote:
> >
> > 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.
>
> Hi Frank,
>
> Thank you for your feedback. You are right, unchecked memory
> allocation in the kernel would not be a good idea. Your suggested
> approach of page-based xattr seems like a good idea but not something
> I feel like I can take on right now. I wonder if we can lobby for the
> xattr limit to be increased to 1M. That would serve NFS needs as right
> now rpc calls (and thus getattr) are no larger than 1M. Thoughts on
> that? I'm not familiar with generic xattr usage and I wonder if even
> local usage could benefit from having a larger limit.

I guess I found an answer to my own question in
https://www.spinics.net/lists/linux-fsdevel/msg85580.html

"> Can we get rid of the 64k size limit for EAs? The API on AIX is the
same as on
> Linux.  But there is a huge size limit - which would help us already a lot.

No - the maximum xattr size of 64k is encoded into the on-disk
format of many filesystems and that's not a simple thing to change."


>
> >
> > - Frank