[PATCH 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.

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 simple_strtoull() into unsigned long long locals
and fail the write with -EINVAL if any of them exceeds U32_MAX, before
the existing checks, which are kept unchanged and in the same order.
On 32-bit kernels unsigned long is 32 bits wide and the truncation
happens inside simple_strtoul() itself, so a range check on its return
value would never fire there; that is why the values are parsed as
unsigned long long. simple_strtoull() keeps the end pointer behaviour
the multi-line parser relies on.

This is hardening, not a security fix: the truncated values are valid
ids and every later check still applies to them.

Signed-off-by: Natasha Klaus <natalie.klaus@xxxxxxxxxxxxxxxxxxxxxxx>
Assisted-by: LLM
---
checkpatch warns that simple_strtoull() is obsolete. kstrtoull() does not
fit here: the buffer holds several lines of three space-separated fields,
and the parser relies on the end pointer to step from one field to the
next, which kstrto*() does not provide.

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

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0bed462e9b2a..e7ed475c3685 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -940,6 +940,7 @@ static ssize_t map_write(struct file *file, const char __user *buf,
 	struct uid_gid_map new_map;
 	unsigned idx;
 	struct uid_gid_extent extent;
+	unsigned long long first, lower_first, cnt;
 	char *kbuf, *pos, *next_line;
 	ssize_t ret;
 
@@ -1001,17 +1002,17 @@ static ssize_t map_write(struct file *file, const char __user *buf,
 		}
 
 		pos = skip_spaces(pos);
-		extent.first = simple_strtoul(pos, &pos, 10);
+		first = simple_strtoull(pos, &pos, 10);
 		if (!isspace(*pos))
 			goto out;
 
 		pos = skip_spaces(pos);
-		extent.lower_first = simple_strtoul(pos, &pos, 10);
+		lower_first = simple_strtoull(pos, &pos, 10);
 		if (!isspace(*pos))
 			goto out;
 
 		pos = skip_spaces(pos);
-		extent.count = simple_strtoul(pos, &pos, 10);
+		cnt = simple_strtoull(pos, &pos, 10);
 		if (*pos && !isspace(*pos))
 			goto out;
 
@@ -1020,6 +1021,13 @@ static ssize_t map_write(struct file *file, const char __user *buf,
 		if (*pos != '\0')
 			goto out;
 
+		/* Verify the values fit in the u32 fields of the extent */
+		if (first > U32_MAX || lower_first > U32_MAX || cnt > U32_MAX)
+			goto out;
+		extent.first = first;
+		extent.lower_first = lower_first;
+		extent.count = cnt;
+
 		/* Verify we have been given valid starting values */
 		if ((extent.first == (u32) -1) ||
 		    (extent.lower_first == (u32) -1))
-- 
2.34.1