| Message ID | Z1n-Ue19Pa_AWVu0@codewreck.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof) | expand |
On Wed, 11 Dec 2024 at 13:04, <asmadeus@codewreck.org> 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. No? Linus
On Wed, Dec 11, 2024 at 01:32:26PM -0800, Linus Torvalds wrote: > On Wed, 11 Dec 2024 at 13:04, <asmadeus@codewreck.org> 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").
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@codewreck.org> 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. /Christian
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@codewreck.org> 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
diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c index 8604e3377ee7..97f60b73bf16 100644 --- a/fs/9p/xattr.c +++ b/fs/9p/xattr.c @@ -37,8 +37,8 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name, if (attr_size > buffer_size) { if (buffer_size) retval = -ERANGE; - else if (attr_size > SSIZE_MAX) - retval = -EOVERFLOW; + else if (attr_size > KMALLOC_MAX_SIZE) + retval = -E2BIG; else /* request to get the attr_size */ retval = attr_size; } else {