linux-kernel - Re: [PATCH] 9p: cap xattr max size to XATTR_SIZE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZvB9YsV8BZSFoDrC@codewreck.org>
Date: Mon, 23 Sep 2024 05:26:10 +0900
From: Dominique Martinet <asmadeus@...ewreck.org>
To: Fedor Pchelkin <pchelkin@...ras.ru>
Cc: Christian Schoenebeck <linux_oss@...debyte.com>,
	Eric Van Hensbergen <ericvh@...nel.org>,
	Latchesar Ionkov <lucho@...kov.net>, v9fs@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	Christian Brauner <brauner@...nel.org>,
	xingwei lee <xrivendell7@...il.com>,
	sam sun <samsun1006219@...il.com>, lvc-project@...uxtesting.org,
	Alexey Khoroshilov <khoroshilov@...ras.ru>
Subject: Re: [PATCH] 9p: cap xattr max size to XATTR_SIZE_MAX

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