[PATCH v2 0/2] userns: reject out-of-range uid_map/gid_map values

[PATCH v2 0/2] userns: reject out-of-range uid_map/gid_map values

[Natasha Klaus]

map_write() silently truncates uid_map/gid_map/projid_map values above
U32_MAX modulo 2^32 on 64-bit kernels, so e.g. "4294967301 1000 1" is
installed as "5 1000 1" without an error. Patch 1 parses each field
with kstrtou32() so such values fail with -EINVAL. Patch 2 adds
selftests for out-of-range and negative values in each field and for
the unchanged in-range path.

This makes the parser more correct; it is not a security fix.

The patch and the selftest were drafted by Claude Code (Claude Opus 5.5)
in an interactive session; I reviewed, built and tested them. The
assistant flagged the truncation while reading map_write(), and I
confirmed it by writing to uid_map on a running kernel.

Tested on v7.3-rc5-37-g551c722f4080 under virtme-ng, x86_64 defconfig
with CONFIG_USER_NS: all 12 selftest cases pass on the patched kernel.
On an unpatched 6.8 host the four above-u32 cases fail as expected,
seven pass and the root-only case is skipped.

Changes in v2:
- Parse each field with kstrtou32() instead of simple_strtoull() plus a
  separate U32_MAX check (Eric W. Biederman).
- Selftest: add negative values in each field, "0 <uid> 1", the
  largest valid first id, a line with extra spaces between fields, and
  the largest count written by a privileged parent (Eric W. Biederman).
- Describe patch 1 as making the parser more correct, not as hardening.
- Rebased onto v7.3-rc5-37-g551c722f4080.

Link: https://lore.kernel.org/all/20260928072537.115668-1-natalie.klaus@runtimeverification.com/

Natasha Klaus (2):
  userns: reject uid_map/gid_map values that do not fit in u32
  selftests/namespaces: add uid_map/gid_map range tests

 kernel/user_namespace.c                       |  29 +-
 tools/testing/selftests/namespaces/.gitignore |   1 +
 tools/testing/selftests/namespaces/Makefile   |   3 +-
 .../selftests/namespaces/uid_map_range_test.c | 299 ++++++++++++++++++
 4 files changed, 328 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/namespaces/uid_map_range_test.c


base-commit: 551c722f40809618230001baccf219193e22fc5a
-- 
2.34.1

[PATCH v2 1/2] userns: reject uid_map/gid_map values that do not fit in u32

[Natasha Klaus]

map_write() parses each line written to uid_map, gid_map or projid_map
with simple_strtoul() and stores the results in the u32 fields of
struct uid_gid_extent. On 64-bit kernels a value above U32_MAX is
silently truncated modulo 2^32. The coding assistant flagged this
truncation site while reading map_write(), and I confirmed it by hand
on a running kernel, where writing to the uid_map of a fresh user
namespace installs a different mapping than the one requested:

  "4294967296 1000 1"  ->  installed as "0 1000 1"
  "4294967297 1000 1"  ->  installed as "1 1000 1"
  "4294967301 1000 1"  ->  installed as "5 1000 1"

The same happens to the lower_first and count fields. Only a literal of
2^64 or more is rejected, and only because simple_strtoul() saturates
to ULLONG_MAX, which truncates onto the (u32)-1 value map_write()
already refuses. On 32-bit kernels unsigned long is itself 32 bits
wide, so the same truncation happens inside simple_strtoul().

The write succeeds, so a runtime that computes ranges and gets one
wrong ends up with a different map than it asked for and no error to
tell it so.

Parse each field with kstrtou32() instead. A new helper,
map_parse_u32(), terminates the field at the next whitespace or NUL,
parses it and puts the original character back, leaving pos where
simple_strtoul() used to leave it. The checks after each field, the
trailing junk check and every later check are unchanged and run in the
same order. A field that is not a decimal number fitting in u32 now
fails the write with -EINVAL, like any other malformed line.

Besides the range check, the one visible difference is that kstrtou32()
accepts a single leading '+', so "+5 1000 1" is now installed as
"5 1000 1" where it used to be rejected.

This makes the parser more correct; it is not a security fix: the
truncated values are valid ids and every later check still applies to
them.

Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
Assisted-by: LLM
---
kbuf is map_write()'s private copy of the user buffer from
memdup_user_nul(): it is NUL-terminated, nothing else refers to it, and
map_write() already overwrites each '\n' in it with '\0' and frees it
on return. So map_parse_u32() can write a '\0' after each field for
kstrtou32() and put the original character back.

 kernel/user_namespace.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0bed462e9b2a..1e232f78cfd6 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -929,6 +929,26 @@ static bool verify_root_map(const struct file *file,
 	return true;
 }
 
+/*
+ * Parse the decimal u32 at *pos, up to the next whitespace or NUL, and
+ * leave *pos pointing at that character.
+ */
+static int map_parse_u32(char **pos, u32 *res)
+{
+	char *end = *pos;
+	char c;
+	int ret;
+
+	while (*end && !isspace(*end))
+		end++;
+	c = *end;
+	*end = '\0';
+	ret = kstrtou32(*pos, 10, res);
+	*end = c;
+	*pos = end;
+	return ret;
+}
+
 static ssize_t map_write(struct file *file, const char __user *buf,
 			 size_t count, loff_t *ppos,
 			 int cap_setid,
@@ -1001,17 +1021,20 @@ static ssize_t map_write(struct file *file, const char __user *buf,
 		}
 
 		pos = skip_spaces(pos);
-		extent.first = simple_strtoul(pos, &pos, 10);
+		if (map_parse_u32(&pos, &extent.first))
+			goto out;
 		if (!isspace(*pos))
 			goto out;
 
 		pos = skip_spaces(pos);
-		extent.lower_first = simple_strtoul(pos, &pos, 10);
+		if (map_parse_u32(&pos, &extent.lower_first))
+			goto out;
 		if (!isspace(*pos))
 			goto out;
 
 		pos = skip_spaces(pos);
-		extent.count = simple_strtoul(pos, &pos, 10);
+		if (map_parse_u32(&pos, &extent.count))
+			goto out;
 		if (*pos && !isspace(*pos))
 			goto out;
 
-- 
2.34.1

[PATCH v2 2/2] selftests/namespaces: add uid_map/gid_map range tests

[Natasha Klaus]

Check that a uid_map or gid_map line with a value that does not fit in
u32, or with a negative value, is rejected with EINVAL and installs
nothing, for each of the three fields.

Check that in-range lines are still accepted and read back unchanged:
a plain single-id mapping, first id 0, the largest valid first id
4294967294, and a line with extra spaces between the fields. When run
as root in the initial user namespace, also check that the largest
count, "0 0 4294967295", is accepted when written by the parent into
the uid_map of a child namespace; an unprivileged writer can only map
a single id.

Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
Assisted-by: LLM
---
 tools/testing/selftests/namespaces/.gitignore |   1 +
 tools/testing/selftests/namespaces/Makefile   |   3 +-
 .../selftests/namespaces/uid_map_range_test.c | 299 ++++++++++++++++++
 3 files changed, 302 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/namespaces/uid_map_range_test.c

diff --git a/tools/testing/selftests/namespaces/.gitignore b/tools/testing/selftests/namespaces/.gitignore
index 0989e80da457..995b082357fa 100644
--- a/tools/testing/selftests/namespaces/.gitignore
+++ b/tools/testing/selftests/namespaces/.gitignore
@@ -10,3 +10,4 @@ cred_change_test
 stress_test
 listns_pagination_bug
 regression_pidfd_setns_test
+uid_map_range_test
diff --git a/tools/testing/selftests/namespaces/Makefile b/tools/testing/selftests/namespaces/Makefile
index fbb821652c17..9b22d12708e1 100644
--- a/tools/testing/selftests/namespaces/Makefile
+++ b/tools/testing/selftests/namespaces/Makefile
@@ -13,7 +13,8 @@ TEST_GEN_PROGS := nsid_test \
 		  cred_change_test \
 		  stress_test \
 		  listns_pagination_bug \
-		  regression_pidfd_setns_test
+		  regression_pidfd_setns_test \
+		  uid_map_range_test
 
 include ../lib.mk
 
diff --git a/tools/testing/selftests/namespaces/uid_map_range_test.c b/tools/testing/selftests/namespaces/uid_map_range_test.c
new file mode 100644
index 000000000000..1eba1e609205
--- /dev/null
+++ b/tools/testing/selftests/namespaces/uid_map_range_test.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include "../kselftest_harness.h"
+
+/*
+ * The fields of a uid_map/gid_map line are stored in u32. Values that
+ * do not fit, and negative values, must be rejected with EINVAL instead
+ * of being silently truncated, and in-range values must still be
+ * accepted.
+ */
+
+static int write_file(const char *path, const char *buf)
+{
+	ssize_t len = strlen(buf);
+	ssize_t ret;
+	int fd;
+
+	fd = open(path, O_WRONLY | O_CLOEXEC);
+	if (fd < 0)
+		return -errno;
+	ret = write(fd, buf, len);
+	if (ret < 0)
+		ret = -errno;
+	close(fd);
+	if (ret < 0)
+		return ret;
+	return ret == len ? 0 : -EIO;
+}
+
+static ssize_t read_file(const char *path, char *buf, size_t size)
+{
+	ssize_t ret;
+	int fd;
+
+	fd = open(path, O_RDONLY | O_CLOEXEC);
+	if (fd < 0)
+		return -errno;
+	ret = read(fd, buf, size - 1);
+	if (ret < 0)
+		ret = -errno;
+	else
+		buf[ret] = '\0';
+	close(fd);
+	return ret;
+}
+
+/* Enter a fresh user namespace, remembering the outer ids. */
+static int enter_userns(uid_t *uid, gid_t *gid)
+{
+	*uid = geteuid();
+	*gid = getegid();
+	if (unshare(CLONE_NEWUSER) < 0)
+		return -errno;
+	/* Required before an unprivileged gid_map write. */
+	return write_file("/proc/self/setgroups", "deny");
+}
+
+static void expect_rejected(struct __test_metadata *_metadata,
+			    const char *path, const char *line)
+{
+	char buf[256];
+
+	TH_LOG("writing \"%s\" to %s", line, path);
+	EXPECT_EQ(write_file(path, line), -EINVAL);
+	/* Nothing may have been installed. */
+	EXPECT_EQ(read_file(path, buf, sizeof(buf)), 0) {
+		TH_LOG("installed: %s", buf);
+	}
+}
+
+static void expect_accepted(struct __test_metadata *_metadata,
+			    const char *path, const char *line,
+			    unsigned int first, unsigned int lower_first,
+			    unsigned int count)
+{
+	unsigned int f, l, c;
+	char buf[256];
+
+	TH_LOG("writing \"%s\" to %s", line, path);
+	ASSERT_EQ(write_file(path, line), 0);
+
+	ASSERT_GT(read_file(path, buf, sizeof(buf)), 0);
+	ASSERT_EQ(sscanf(buf, "%u %u %u", &f, &l, &c), 3);
+	EXPECT_EQ(f, first);
+	EXPECT_EQ(l, lower_first);
+	EXPECT_EQ(c, count);
+	/* Exactly one extent. */
+	EXPECT_EQ(strchr(buf, '\n'), buf + strlen(buf) - 1);
+}
+
+TEST(uid_map_first_above_u32)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "4294967301 %u 1", uid);
+	expect_rejected(_metadata, "/proc/self/uid_map", line);
+}
+
+TEST(uid_map_lower_first_above_u32)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "0 %llu 1", (1ULL << 32) + uid);
+	expect_rejected(_metadata, "/proc/self/uid_map", line);
+}
+
+TEST(uid_map_count_above_u32)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "0 %u 4294967297", uid);
+	expect_rejected(_metadata, "/proc/self/uid_map", line);
+}
+
+TEST(gid_map_first_above_u32)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "4294967301 %u 1", gid);
+	expect_rejected(_metadata, "/proc/self/gid_map", line);
+}
+
+TEST(uid_map_first_negative)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "-1 %u 1", uid);
+	expect_rejected(_metadata, "/proc/self/uid_map", line);
+}
+
+TEST(uid_map_lower_first_negative)
+{
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	expect_rejected(_metadata, "/proc/self/uid_map", "0 -1 1");
+}
+
+TEST(uid_map_count_negative)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "0 %u -1", uid);
+	expect_rejected(_metadata, "/proc/self/uid_map", line);
+}
+
+TEST(uid_map_in_range)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "5 %u 1", uid);
+	expect_accepted(_metadata, "/proc/self/uid_map", line, 5, uid, 1);
+}
+
+TEST(uid_map_zero)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "0 %u 1", uid);
+	expect_accepted(_metadata, "/proc/self/uid_map", line, 0, uid, 1);
+}
+
+/* 4294967295 is (u32)-1, which map_write() refuses as a starting id. */
+TEST(uid_map_largest_first)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "4294967294 %u 1", uid);
+	expect_accepted(_metadata, "/proc/self/uid_map", line,
+			4294967294U, uid, 1);
+}
+
+TEST(uid_map_extra_spaces)
+{
+	char line[64];
+	uid_t uid;
+	gid_t gid;
+	int ret;
+
+	ret = enter_userns(&uid, &gid);
+	if (ret)
+		SKIP(return, "cannot set up user namespace: %s", strerror(-ret));
+	snprintf(line, sizeof(line), "0   %u  1", uid);
+	expect_accepted(_metadata, "/proc/self/uid_map", line, 0, uid, 1);
+}
+
+/*
+ * An unprivileged writer may only map a single id, so the largest count
+ * needs a privileged parent writing the map of a child namespace. With
+ * first and lower_first 0, the no-wrap rule allows up to 4294967295.
+ */
+TEST(uid_map_largest_count)
+{
+	unsigned int first, lower_first, count;
+	int to_child[2], from_child[2];
+	char path[64], buf[256];
+	pid_t pid;
+	char c;
+
+	if (geteuid() != 0)
+		SKIP(return, "needs root in the initial user namespace");
+	ASSERT_GT(read_file("/proc/self/uid_map", buf, sizeof(buf)), 0);
+	ASSERT_EQ(sscanf(buf, "%u %u %u", &first, &lower_first, &count), 3);
+	if (first != 0 || lower_first != 0 || count != 4294967295U)
+		SKIP(return, "not in the initial user namespace");
+
+	ASSERT_EQ(pipe(to_child), 0);
+	ASSERT_EQ(pipe(from_child), 0);
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0) {
+		close(to_child[1]);
+		close(from_child[0]);
+		c = unshare(CLONE_NEWUSER) ? 'F' : 'K';
+		if (write(from_child[1], &c, 1) != 1)
+			_exit(1);
+		/* Stay alive until the parent is done with our uid_map. */
+		if (read(to_child[0], &c, 1) < 0)
+			_exit(1);
+		_exit(0);
+	}
+	close(to_child[0]);
+	close(from_child[1]);
+
+	ASSERT_EQ(read(from_child[0], &c, 1), 1);
+	ASSERT_EQ(c, 'K');
+	snprintf(path, sizeof(path), "/proc/%d/uid_map", pid);
+	expect_accepted(_metadata, path, "0 0 4294967295", 0, 0, 4294967295U);
+
+	close(to_child[1]);
+	EXPECT_EQ(waitpid(pid, NULL, 0), pid);
+}
+
+TEST_HARNESS_MAIN
-- 
2.34.1