Re: [PATCH] 9p: cap xattr max size to XATTR_SIZE_MAX

[Dominique Martinet <asmadeus@codewreck.org>]

Fedor Pchelkin wrote on Sun, Sep 01, 2024 at 10:16:55AM +0300:
> the issue is still present in upstream kernel [1].

Right, I didn't register the end of discussion, thanks for the reminder

> Considering the remark from Christian that limiting the allocation to
> XATTR_SIZE_MAX seems too Linux-specific, maybe just fail silently with
> __GFP_NOWARN flag passed to the allocator and return ENOMEM? I submitted
> the patch [2] sometime ago which looks still applicable to the mainline
> kernel. It was superseded with current discussion.

Hmm, at high level, I don't really see the problem of being
linux-specific here - we're writing a linux client.
Something larger might work in the very specific case of the acl call
here, but normal userspace getxattr will be truncated to XATTR_SIZE_MAX
anyway (in do_getxattr), and returning a size larger than that will
yield odd behavior for userspace: calling getxattr with a zero size will
return a size larger than XATTR_SIZE_MAX, then trying to call getxattr
with the returned size will just fail as the buffer is silently
truncated to a shorter size without any way to work around that; it
sounds more coherent to me to just say we can't do it in the first
place.

Anyway, I guess a malicious server has an infinite number of way to
cause nuisance to our clients here so this is the least of our worry,
and not giving a warning is better than giving one, so if we're not
doing anything else adding NOWARN is probably for the best but I'm
really not sure I agree there...

I'm getting less involved with linux as real life commitments increase,
so I'll just defer to whatever eminent folks think about here --
Christian (Brauner), please take your pick of either of these (or
anything else) and I'll apply it:
(just adding nowarn to the acl path that warned)
https://lore.kernel.org/lkml/20240202121319.21743-1-pchelkin@ispras.ru/
or
(limit to XATTR_SIZE_MAX in 9p's xattr_handler->get() op)
https://lore.kernel.org/all/20240304-xattr_maxsize-v1-1-322357ec6bdf@codewreck.org/



(and I see vfs_getxattr_alloc() hasn't been updated to error on
size > XATTR_SIZE_MAX either, but we don't use it anyway)

Thanks,
-- 
Dominique Martinet | Asmadeus