Repository navigation
fs: fixes for statmount() short buffers and listmount()/listns() cursors - #2935
Open
vfsci-bot[bot] wants to merge 6 commits into
Open
vfsci-bot[bot] wants to merge 6 commits into
vfsci-bot[bot] wants to merge 6 commits into
Conversation
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 e52e97f ("statmount: let unset strings be empty"). If the handler emits nothing the early return from commit 75ead69 ("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: e52e97f ("statmount: let unset strings be empty") Cc: stable@vger.kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> Reviewed-by: Jeff Layton <jlayton@kernel.org>
Request strings that end up empty with buffers that aren't larger than the fixed part of struct statmount and check that: - every request fails with EOVERFLOW - nothing is written past the end of the buffer, a guard page right behind it catches that The buffer sizes run from zero up to sizeof(struct statmount) and the requests cover STATMOUNT_FS_TYPE, STATMOUNT_FS_SUBTYPE and STATMOUNT_MNT_UIDMAP. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
do_listmount() takes the last mount id the caller saw and looks up the
first mount after it by searching for last_mnt_id + 1. With U64_MAX
that wraps around to zero and the search finds the first mount in the
namespace. So a cursor at the largest possible id restarts the listing
from the beginning instead of ending it. A cursor of U64_MAX - 1
correctly returns nothing.
No mount will ever have that id. Mount ids come from a counter that
starts at MNT_UNIQUE_ID_OFFSET + 1 and a caller that only feeds back
ids it got from us can't hit this. It's still wrong. Nothing can come
after the largest possible id so return no entries for it, just like
for any other cursor past the last mount.
The reverse direction isn't affected. A cursor of zero is special-cased
and anything else is larger than MNT_UNIQUE_ID_OFFSET so last_mnt_id - 1
can't underflow.
Reproducer, unprivileged:
#define _GNU_SOURCE
#include <errno.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>
#define LSMT_ROOT 0xffffffffffffffffULL
struct mnt_id_req {
uint32_t size, mnt_ns_fd;
uint64_t mnt_id, param, mnt_ns_id;
};
static void list(uint64_t cursor)
{
struct mnt_id_req req = {
.size = sizeof(req), .mnt_id = LSMT_ROOT, .param = cursor,
};
uint64_t ids[8];
long ret;
errno = 0;
ret = syscall(__NR_listmount, &req, ids, 8, 0);
printf("cursor %llu: ret %ld errno %d", (unsigned long long)cursor,
ret, errno);
for (long i = 0; i < ret && i < 1; i++)
printf("%s%llu", i ? "," : " ids ", (unsigned long long)ids[i]);
printf("%s\n", ret > 1 ? ",..." : "");
}
int main(void)
{
list(0);
list(UINT64_MAX - 1);
list(UINT64_MAX);
return 0;
}
Before, on 7.3.0-rc7-00252-g4e3259b08c54 (vfs.fixes) in a VM:
cursor 0: ret 5 errno 0 ids 2147483651,...
cursor 18446744073709551614: ret 0 errno 0
cursor 18446744073709551615: ret 5 errno 0 ids 2147483651,...
After:
cursor 0: ret 5 errno 0 ids 2147483651,...
cursor 18446744073709551614: ret 0 errno 0
cursor 18446744073709551615: ret 0 errno 0
Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/r/20261020110732.959552-1-shawnq1@163.com
Fixes: b4c2bea ("add listmount(2) syscall")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Pass the largest possible mount id as the cursor and check that listmount() returns no entries instead of starting over at the first mount. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
do_listns() and do_listns_userns() take the last namespace id the caller
saw and look up the first namespace after it by searching the rbtree for
last_ns_id + 1. With U64_MAX that wraps around to zero and the search
returns the namespace with the smallest id. So a cursor at the largest
possible id restarts the listing from the beginning instead of failing
with ENOENT like a cursor of U64_MAX - 1 does. That's the case for the
unified tree, the type-specific trees and the owner tree alike.
No namespace will ever have that id. They come from a counter and a
caller that only feeds back ids it got from us can't hit this. It's
still wrong.
Instead of adding one to the cursor and searching for the smallest id
that's greater or equal make the lookup helpers search for the smallest
id that's strictly greater than the cursor. That's what both callers
want anyway and there's nothing left to overflow.
Reproducer, unprivileged:
#define _GNU_SOURCE
#include <errno.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>
#ifndef __NR_listns
#define __NR_listns 470
#endif
#define CLONE_NEWUSER 0x10000000U
#define LISTNS_CURRENT_USER 0xffffffffffffffffULL
struct ns_id_req {
uint32_t size, spare;
uint64_t ns_id;
uint32_t ns_type, spare2;
uint64_t user_ns_id;
};
static void list(const char *tree, uint32_t type, uint64_t owner,
uint64_t cursor)
{
struct ns_id_req req = {
.size = sizeof(req), .ns_id = cursor, .ns_type = type,
.user_ns_id = owner,
};
uint64_t ids[8];
long ret;
errno = 0;
ret = syscall(__NR_listns, &req, ids, 8, 0);
printf("%s cursor %llu: ret %ld errno %d", tree,
(unsigned long long)cursor, ret, errno);
for (long i = 0; i < ret && i < 3; i++)
printf("%s%llu", i ? "," : " ids ", (unsigned long long)ids[i]);
printf("%s\n", ret > 3 ? ",..." : "");
}
int main(void)
{
static const uint64_t cursors[] = { 0, UINT64_MAX - 1, UINT64_MAX };
for (int i = 0; i < 3; i++)
list("unified", 0, 0, cursors[i]);
for (int i = 0; i < 3; i++)
list("userns", CLONE_NEWUSER, 0, cursors[i]);
for (int i = 0; i < 3; i++)
list("owner", 0, LISTNS_CURRENT_USER, cursors[i]);
return 0;
}
Before, on 7.3.0-rc7-00252-g4e3259b08c54 (vfs.fixes) in a VM:
unified cursor 0: ret 8 errno 0 ids 1,2,3,...
unified cursor 18446744073709551614: ret -1 errno 2
unified cursor 18446744073709551615: ret 8 errno 0 ids 1,2,3,...
userns cursor 0: ret 1 errno 0 ids 3
userns cursor 18446744073709551614: ret -1 errno 2
userns cursor 18446744073709551615: ret 1 errno 0 ids 3
owner cursor 0: ret 8 errno 0 ids 1,2,4,...
owner cursor 18446744073709551614: ret -1 errno 2
owner cursor 18446744073709551615: ret 8 errno 0 ids 1,2,4,...
After:
unified cursor 0: ret 8 errno 0 ids 1,2,3,...
unified cursor 18446744073709551614: ret -1 errno 2
unified cursor 18446744073709551615: ret -1 errno 2
userns cursor 0: ret 1 errno 0 ids 3
userns cursor 18446744073709551614: ret -1 errno 2
userns cursor 18446744073709551615: ret -1 errno 2
owner cursor 0: ret 8 errno 0 ids 1,2,4,...
owner cursor 18446744073709551614: ret -1 errno 2
owner cursor 18446744073709551615: ret -1 errno 2
Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/r/20261020110732.959552-1-shawnq1@163.com
Fixes: 76b6f5d ("nstree: add listns()")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Pass the largest possible namespace id as the cursor and check that listns() fails with ENOENT instead of starting over: - in the unified tree - in a type-specific tree - in the owner tree of the caller's user namespace Nothing can come after the largest possible id. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Series: https://patchwork.kernel.org/project/linux-fsdevel/list/?series=1190450
Submitter: Christian Brauner
Version: 1
Patches: 6/6
Message-ID:
<20261021-work-mount-statmount_listns_fixes-v1-0-8917a4c1bd14@kernel.org>Base: vfs.base.ci
Lore: https://lore.kernel.org/linux-fsdevel/20261021-work-mount-statmount_listns_fixes-v1-0-8917a4c1bd14@kernel.org
Automated by ml2pr