Re: Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof)

[Christian Schoenebeck]

On Thursday, December 12, 2024 11:17:06 AM CET Christian Schoenebeck wrote:
> On Wednesday, December 11, 2024 11:55:00 PM CET Al Viro wrote:
> > On Wed, Dec 11, 2024 at 01:32:26PM -0800, Linus Torvalds wrote:
> > > On Wed, 11 Dec 2024 at 13:04, <asmadeus@xxxxxxxxxxxxx> wrote:
> > > >
> > > > Christian Schoenebeck's suggestion was something like this -- I guess
> > > > that's good enough for now and won't break anything (e.g. ACLs bigger
> > > > than XATTR_SIZE_MAX), so shall we go with that instead?
> > > 
> > > Please use XATTR_SIZE_MAX. The KMALLOC_MAX_SIZE limit seems to make no
> > > sense in this context.
> > > 
> > > Afaik the VFS layer doesn't allow getting an xattr bigger than
> > > XATTR_SIZE_MAX anyway, and would return E2BIG for them later
> > > regardless, so returning anything bigger wouldn't work anyway, even if
> > > p9 tried to return such a thing up to some bigger limit.
> > 
> > E2BIG on attempt to set, quiet cap to XATTR_SIZE_MAX on attempt to get
> > (i.e. never asking more than that from fs) and if filesystem complains
> > about XATTR_SIZE_MAX not being enough, E2BIG it is (instead of ERANGE
> > normally expected on "your buffer is too small for that").
> 
> So that cap is effective even if that xattr does not go out to user space?
> 
> I mean the concern I had was about ACLs on guest, which are often mapped with 
> 9p to xattr on host and can become pretty big. So these were xattr not 
> directly exposed to guest's user space.

AFAICS it is not capped in this particular case: v9fs_fid_get_acl() calls
v9fs_fid_xattr_get() for getting the xattr, which in turn calls p9 client
functions to retrieve the xattr directly from 9p server (host). So the regular
Linux VFS layers are not involved here.

I also see no limit applied in fs/posix_acl.c when encoding/decoding ACLs.

And 9p server is not necessarily a Linux host, hence Linux's limit for xattr
do not necessarily apply.

So to me KMALLOC_MAX_SIZE (or better: 9p client's msize - header) still looks
right here, no?

/Christian