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 <[email protected]> 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
