Merge patch series "fs: fixes for statmount() short buffers and listmount()/listns() cursors"

Christian Brauner <brauner@kernel.org> says:

shaos reported two issues in [1] and [2]. Both are real.

- statmount() reserves the empty string in the seq buffer before a
  handler runs but the early return for "nothing emitted" never checks
  that byte against the caller's buffer. So ask for STATMOUNT_MNT_UIDMAP
  on a mount that isn't idmapped with a buffer smaller than struct
  statmount and we write one zero byte past the buffer. It's the
  caller's own memory but that's still not ok. I'm fixing this in
  prepare_kstatmount() where the exact-size check already lives so that
  unset strings keep pointing at an empty string.

- listmount() and listns() add one to the cursor before they look up
  the next entry. With U64_MAX that wraps to zero and the listing starts
  over. No real cursor can ever be U64_MAX so this only bites callers
  that make up cursors but it's still wrong.

All three come with selftests. I've tagged the two cursor fixes for
stable as well since they're trivial but I don't feel strongly about
that.

[1]: https://lore.kernel.org/r/20261020110747.959829-1-shawnq1@163.com
[2]: https://lore.kernel.org/r/20261020110732.959552-1-shawnq1@163.com

* patches from https://patch.msgid.link/20261021-work-mount-statmount_listns_fixes-v1-0-8917a4c1bd14@kernel.org:
  selftests/namespaces: check listns() with a U64_MAX cursor
  listns: don't wrap around with a U64_MAX cursor
  selftests/filesystems: check listmount() with a U64_MAX cursor
  listmount: don't wrap around with a U64_MAX cursor
  selftests/filesystems: check short buffers for statmount() strings
  statmount: don't copy the reserved empty string past the buffer

Link: https://patch.msgid.link/20261021-work-mount-statmount_listns_fixes-v1-0-8917a4c1bd14@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

fs/namespace.c

@@ -6067,7 +6067,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);
@@ -6261,6 +6262,8 @@ static ssize_t do_listmount(struct klistmount *kls, bool reverse)
 	} else {
 		if (reverse)
 			first = mnt_find_id_at_reverse(ns, last_mnt_id - 1);
+		else if (last_mnt_id == U64_MAX)
+			first = NULL; /* nothing comes after the largest id */
 		else
 			first = mnt_find_id_at(ns, last_mnt_id + 1);
 	}

kernel/nstree.c

@@ -459,10 +459,10 @@ static inline int prepare_klistns(struct klistns *kls, struct ns_id_req *kreq,
 }
 
 /*
- * Lookup a namespace owned by owner with id >= ns_id.
- * Returns the namespace with the smallest id that is >= ns_id.
+ * Lookup a namespace owned by owner with id > ns_id.
+ * Returns the namespace with the smallest id that is > ns_id.
  */
-static struct ns_common *lookup_ns_owner_at(u64 ns_id, struct ns_common *owner)
+static struct ns_common *lookup_ns_owner_after(u64 ns_id, struct ns_common *owner)
 {
 	struct ns_common *ret = NULL;
 	struct rb_node *node;
@@ -476,10 +476,8 @@ static struct ns_common *lookup_ns_owner_at(u64 ns_id, struct ns_common *owner)
 		struct ns_common *ns;
 
 		ns = node_to_ns_owner(node);
-		if (ns_id <= ns->ns_id) {
+		if (ns_id < ns->ns_id) {
 			ret = ns;
-			if (ns_id == ns->ns_id)
-				break;
 			node = node->rb_left;
 		} else {
 			node = node->rb_right;
@@ -565,7 +563,7 @@ static ssize_t do_listns_userns(struct klistns *kls)
 	 * then use it's list entry to iterate from there.
 	 */
 	if (kls->last_ns_id) {
-		kls->first_ns = lookup_ns_owner_at(kls->last_ns_id + 1, ns);
+		kls->first_ns = lookup_ns_owner_after(kls->last_ns_id, ns);
 		if (!kls->first_ns)
 			return -ENOENT;
 		first_ns = kls->first_ns;
@@ -613,10 +611,10 @@ static ssize_t do_listns_userns(struct klistns *kls)
 }
 
 /*
- * Lookup a namespace with id >= ns_id in either the unified tree or a type-specific tree.
- * Returns the namespace with the smallest id that is >= ns_id.
+ * Lookup a namespace with id > ns_id in either the unified tree or a type-specific tree.
+ * Returns the namespace with the smallest id that is > ns_id.
  */
-static struct ns_common *lookup_ns_id_at(u64 ns_id, int ns_type)
+static struct ns_common *lookup_ns_id_after(u64 ns_id, int ns_type)
 {
 	struct ns_common *ret = NULL;
 	struct ns_tree_root *ns_tree = NULL;
@@ -643,13 +641,8 @@ static struct ns_common *lookup_ns_id_at(u64 ns_id, int ns_type)
 		else
 			ns = node_to_ns_unified(node);
 
-		if (ns_id <= ns->ns_id) {
-			if (ns_type)
-				ret = node_to_ns(node);
-			else
-				ret = node_to_ns_unified(node);
-			if (ns_id == ns->ns_id)
-				break;
+		if (ns_id < ns->ns_id) {
+			ret = ns;
 			node = node->rb_left;
 		} else {
 			node = node->rb_right;
@@ -708,7 +701,7 @@ static ssize_t do_listns(struct klistns *kls)
 	}
 
 	if (kls->last_ns_id) {
-		kls->first_ns = lookup_ns_id_at(kls->last_ns_id + 1, ns_type);
+		kls->first_ns = lookup_ns_id_after(kls->last_ns_id, ns_type);
 		if (!kls->first_ns)
 			return -ENOENT;
 		first_ns = kls->first_ns;

tools/testing/selftests/filesystems/statmount/listmount_test.c

@@ -63,4 +63,15 @@ TEST(listmount_backward)
 	}
 }
 
+/* A cursor at the largest possible mount id must not wrap around. */
+TEST(listmount_cursor_max)
+{
+	uint64_t list[LISTMNT_BUFFER];
+	ssize_t nr_mounts;
+
+	nr_mounts = listmount(LSMT_ROOT, 0, UINT64_MAX, list,
+			      LISTMNT_BUFFER, 0);
+	ASSERT_EQ(nr_mounts, 0);
+}
+
 TEST_HARNESS_MAIN

tools/testing/selftests/filesystems/statmount/statmount_test.c

@@ -7,6 +7,7 @@
 #include <sched.h>
 #include <fcntl.h>
 #include <sys/param.h>
+#include <sys/mman.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/statfs.h>
@@ -578,6 +579,63 @@ static void test_statmount_string(uint64_t mask, size_t off, const char *name)
 	free(sm);
 }
 
+/*
+ * A string request needs a buffer that is larger than the fixed part of
+ * struct statmount. That's true even if the string turns out to be empty
+ * since the empty string itself is placed past the fixed part.
+ */
+static void test_statmount_string_short_buffer(void)
+{
+	const struct {
+		uint64_t mask;
+		const char *name;
+	} reqs[] = {
+		{ STATMOUNT_FS_TYPE,	"fs type" },
+		{ STATMOUNT_FS_SUBTYPE,	"fs subtype" },
+		{ STATMOUNT_MNT_UIDMAP,	"mount uidmap" },
+	};
+	const size_t sizes[] = { 0, 1, 100, sizeof(struct statmount) - 1,
+				 sizeof(struct statmount) };
+	long page = sysconf(_SC_PAGESIZE);
+	unsigned char *map;
+	size_t i, j;
+	int ret;
+
+	map = mmap(NULL, 2 * page, PROT_READ | PROT_WRITE,
+		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (map == MAP_FAILED) {
+		ksft_test_result_fail("mmap: %s\n", strerror(errno));
+		return;
+	}
+
+	/* Anything written past the end of the buffer faults. */
+	if (mprotect(map + page, page, PROT_NONE)) {
+		ksft_test_result_fail("mprotect: %s\n", strerror(errno));
+		goto out;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(reqs); i++) {
+		for (j = 0; j < ARRAY_SIZE(sizes); j++) {
+			/* The buffer ends right at the guard page. */
+			struct statmount *sm = (void *)(map + page - sizes[j]);
+
+			errno = 0;
+			ret = statmount(root_id, 0, 0, reqs[i].mask, sm,
+					sizes[j], 0);
+			if (ret != -1 || errno != EOVERFLOW) {
+				ksft_test_result_fail("statmount %s with %zu byte buffer: %s\n",
+						      reqs[i].name, sizes[j],
+						      ret ? strerror(errno) : "success");
+				goto out;
+			}
+		}
+	}
+
+	ksft_test_result_pass("statmount string short buffer\n");
+out:
+	munmap(map, 2 * page);
+}
+
 static void test_listmount_tree(void)
 {
 	ssize_t res;
@@ -869,7 +927,7 @@ int main(void)
 
 	setup_namespace();
 
-	ksft_set_plan(17);
+	ksft_set_plan(18);
 	test_listmount_empty_root();
 	test_statmount_zero_mask();
 	test_statmount_mnt_basic();
@@ -884,6 +942,7 @@ int main(void)
 	test_statmount_string(all_mask, str_off(mnt_root), "mount root & all");
 	test_statmount_string(all_mask, str_off(mnt_point), "mount point & all");
 	test_statmount_string(all_mask, str_off(fs_type), "fs type & all");
+	test_statmount_string_short_buffer();
 
 	test_listmount_tree();
 	test_statmount_by_fd_unmounted();

tools/testing/selftests/namespaces/listns_test.c

@@ -676,4 +676,38 @@ TEST(listns_error_cases)
 	}
 }
 
+/*
+ * A cursor at the largest possible namespace id must not wrap around and
+ * restart the listing from the beginning.
+ */
+TEST(listns_cursor_max)
+{
+	struct ns_id_req req = {
+		.size = sizeof(req),
+		.ns_id = ULLONG_MAX,
+	};
+	__u64 ns_ids[10];
+	ssize_t ret;
+
+	/* unified tree */
+	ret = sys_listns(&req, ns_ids, ARRAY_SIZE(ns_ids), 0);
+	if (ret < 0 && errno == ENOSYS)
+		SKIP(return, "listns() not supported");
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, ENOENT);
+
+	/* type-specific tree */
+	req.ns_type = CLONE_NEWUSER;
+	ret = sys_listns(&req, ns_ids, ARRAY_SIZE(ns_ids), 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, ENOENT);
+
+	/* owner tree */
+	req.ns_type = 0;
+	req.user_ns_id = LISTNS_CURRENT_USER;
+	ret = sys_listns(&req, ns_ids, ARRAY_SIZE(ns_ids), 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, ENOENT);
+}
+
 TEST_HARNESS_MAIN