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 <[email protected]> Signed-off-by: Natasha Klaus <[email protected]> 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
