mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: linux-fsdevel@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
	 Miklos Szeredi <mszeredi@redhat.com>,
	Jeff Layton <jlayton@kernel.org>,  shaos <shawnq1@163.com>,
	linux-kernel@vger.kernel.org,  stable@vger.kernel.org,
	"Christian Brauner (Amutable)" <brauner@kernel.org>
Subject: [PATCH 1/6] statmount: don't copy the reserved empty string past the buffer
Date: Wed, 21 Oct 2026 13:46:54 +0200	[thread overview]
Message-ID: <20261021-work-mount-statmount_listns_fixes-v1-1-8917a4c1bd14@kernel.org> (raw)
In-Reply-To: <20261021-work-mount-statmount_listns_fixes-v1-0-8917a4c1bd14@kernel.org>

statmount_string() reserves an empty string at the start of the seq
buffer before it calls the handler for the requested string. That's
what the offsets of unset strings point to since commit e52e97f09fb6
("statmount: let unset strings be empty").

If the handler emits nothing the early return from commit 75ead69a7173
("fs: don't let statmount return empty strings") skips the size check
against the caller's buffer. Back then an empty emission meant an empty
seq buffer and there was nothing to copy. Now the reserved NUL byte is
there and copy_statmount_to_user() copies it to buf + sizeof(struct
statmount) no matter how small bufsize is.

So request a string that may turn out empty with a buffer that's
smaller than struct statmount and the kernel writes one zero byte past
the caller's buffer and reports a size that's larger than the buffer.
STATMOUNT_MNT_UIDMAP or STATMOUNT_MNT_GIDMAP on a mount that isn't
idmapped will do, so will STATMOUNT_FS_SUBTYPE on anything but fuse or
STATMOUNT_OPT_ARRAY on a filesystem without ->show_options(). None of
this needs privileges. The byte only lands in the caller's own address
space but that's still not ok.

prepare_kstatmount() already refuses a buffer of exactly sizeof(struct
statmount) for string requests. A smaller buffer can't hold a string
either and it can't even hold the reserved empty string. So refuse any
buffer that isn't larger than the fixed part. That's also what the uapi
header says: string requests need space after the fixed part of the
structure.

Reproducer, unprivileged, on any mount that isn't idmapped:

    #define _GNU_SOURCE
    #include <errno.h>
    #include <fcntl.h>
    #include <stdint.h>
    #include <stdio.h>
    #include <string.h>
    #include <sys/mman.h>
    #include <sys/syscall.h>
    #include <unistd.h>
    #define STATMOUNT_FS_SUBTYPE	0x00000100U
    #define STATMOUNT_MNT_UIDMAP	0x00002000U
    #define STATX_MNT_ID_UNIQUE	0x00004000U
    #define SM_SIZE			512 /* sizeof(struct statmount) */
    struct mnt_id_req {
    	uint32_t size, mnt_ns_fd;
    	uint64_t mnt_id, param, mnt_ns_id;
    };
    static uint64_t root_mnt_id(void)
    {
    	unsigned char stx[256]; /* struct statx, stx_mnt_id at offset 144 */
    	uint64_t id;
    	if (syscall(__NR_statx, AT_FDCWD, "/", 0, STATX_MNT_ID_UNIQUE, stx))
    		_exit(1);
    	memcpy(&id, stx + 144, sizeof(id));
    	return id;
    }
    static void probe(uint64_t mnt_id, uint64_t mask, int guard)
    {
    	struct mnt_id_req req = {
    		.size = sizeof(req), .mnt_id = mnt_id, .param = mask,
    	};
    	long page = sysconf(_SC_PAGESIZE);
    	unsigned char *map, *buf;
    	uint32_t size;
    	long ret;
    	map = mmap(NULL, 2 * page, PROT_READ | PROT_WRITE,
    		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    	memset(map, 0xaa, 2 * page);
    	/* 100 byte buffer ending right at the page boundary */
    	buf = map + page - 100;
    	if (guard)
    		mprotect(map + page, page, PROT_NONE);
    	errno = 0;
    	ret = syscall(__NR_statmount, &req, buf, 100, 0);
    	printf("mask 0x%04llx%s: ret %ld errno %d", (unsigned long long)mask,
    	       guard ? " guard page" : "", ret, errno);
    	if (!guard) {
    		memcpy(&size, buf, sizeof(size));
    		printf(", size 0x%x, byte[%d] 0x%02x", size, SM_SIZE,
    		       buf[SM_SIZE]);
    	}
    	printf("\n");
    	munmap(map, 2 * page);
    }
    int main(void)
    {
    	uint64_t id = root_mnt_id();
    	probe(id, STATMOUNT_MNT_UIDMAP, 0);
    	probe(id, STATMOUNT_FS_SUBTYPE, 0);
    	probe(id, STATMOUNT_MNT_UIDMAP, 1);
    	return 0;
    }

Before, on 7.3.0-rc7-00252-g4e3259b08c54 (vfs.fixes) in a VM, the
same on a distro 7.1.13:

    mask 0x2000: ret 0 errno 0, size 0x65, byte[512] 0x00
    mask 0x0100: ret 0 errno 0, size 0x65, byte[512] 0x00
    mask 0x2000 guard page: ret -1 errno 14

After:

    mask 0x2000: ret -1 errno 75, size 0xaaaaaaaa, byte[512] 0xaa
    mask 0x0100: ret -1 errno 75, size 0xaaaaaaaa, byte[512] 0xaa
    mask 0x2000 guard page: ret -1 errno 75

Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/r/20261020110747.959829-1-shawnq1@163.com
Fixes: e52e97f09fb6 ("statmount: let unset strings be empty")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/namespace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index c30f063742f4..6c9c0cdbe890 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -6058,7 +6058,8 @@ static int prepare_kstatmount(struct kstatmount *ks, struct mnt_id_req *kreq,
 	ks->bufsize = bufsize;
 
 	if (ks->mask & STATMOUNT_STRING_REQ) {
-		if (bufsize == sizeof(ks->sm))
+		/* Even the reserved empty string needs room past the fixed part. */
+		if (bufsize <= sizeof(ks->sm))
 			return -EOVERFLOW;
 
 		ks->seq.buf = kvmalloc(seq_size, GFP_KERNEL_ACCOUNT);

-- 
2.53.0


  reply	other threads:[~2026-10-21 11:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 11:46 [PATCH 0/6] fs: fixes for statmount() short buffers and listmount()/listns() cursors Christian Brauner
2026-10-21 11:46 ` Christian Brauner [this message]
2026-10-21 11:52   ` [PATCH 1/6] statmount: don't copy the reserved empty string past the buffer Jeff Layton
2026-10-21 15:18   ` Jan Kara
2026-10-21 11:46 ` [PATCH 2/6] selftests/filesystems: check short buffers for statmount() strings Christian Brauner
2026-10-21 11:46 ` [PATCH 3/6] listmount: don't wrap around with a U64_MAX cursor Christian Brauner
2026-10-21 11:52   ` Jeff Layton
2026-10-21 15:25   ` Jan Kara
2026-10-21 11:46 ` [PATCH 4/6] selftests/filesystems: check listmount() " Christian Brauner
2026-10-21 11:46 ` [PATCH 5/6] listns: don't wrap around " Christian Brauner
2026-10-21 11:55   ` Jeff Layton
2026-10-21 15:29   ` Jan Kara
2026-10-21 11:46 ` [PATCH 6/6] selftests/namespaces: check listns() " Christian Brauner
2026-10-21 13:18 ` [PATCH 0/6] fs: fixes for statmount() short buffers and listmount()/listns() cursors Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261021-work-mount-statmount_listns_fixes-v1-1-8917a4c1bd14@kernel.org \
    --to=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=shawnq1@163.com \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®