linux-kernel - Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z1n-Ue19Pa_AWVu0@codewreck.org>
Date: Thu, 12 Dec 2024 06:04:17 +0900
From: asmadeus@...ewreck.org
To: Leo Stone <leocstone@...il.com>
Cc: syzbot+03fb58296859d8dbab4d@...kaller.appspotmail.com, ericvh@...il.com,
	ericvh@...nel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux_oss@...debyte.com,
	lucho@...kov.net, syzkaller-bugs@...glegroups.com,
	torvalds@...ux-foundation.org, v9fs-developer@...ts.sourceforge.net,
	v9fs@...ts.linux.dev, viro@...iv.linux.org.uk,
	Fedor Pchelkin <pchelkin@...ras.ru>,
	Seth Forshee <sforshee@...nel.org>,
	Christian Brauner <brauner@...nel.org>
Subject: Alloc cap limit for 9p xattrs (Was: WARNING in
 __alloc_frozen_pages_noprof)

Leo Stone wrote on Wed, Dec 11, 2024 at 12:02:40PM -0800:
> syzbot creates a pipe and writes some data to it. It then creates a v9fs
> mount using the pipe as transport. The data in the pipe specifies an ACL
> of size 9 TB (9895604649984 bytes) for the root inode, causing kmalloc
> to fail.

grmbl.

Sorry about that, there's been some paches ages ago to either cap xattrs
allocations to XATTR_SIZE_MAX, KMALLOC_MAX_SIZE, look into
vfs_getxattr_alloc or just flag the alloc __GFP_NOWARN:
https://lore.kernel.org/all/20240304-xattr_maxsize-v1-1-322357ec6bdf@codewreck.org/T/#u

and it was left forgotten because no decision was taken on something I
don't have time to think about

I've re-added everyone involved in Ccs, let's pick one and be done with
it.

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?

I don't care but let's get something in this cycle, the first patch is
almost one year old and this is ridiculous...

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 {

--
Dominique,
sleepy