[PATCH] init/do_mounts.c: Harden split_fs_names() against buffer overflow

From: Vivek Goyal
Date: Wed Sep 15 2021 - 11:22:38 EST


split_fs_names() currently takes comma separated list of filesystems
and converts it into individual filesystem strings. Pleaces these
strings in the input buffer passed by caller and returns number of
strings.

If caller manages to pass input string bigger than buffer, then we
can write beyond the buffer. Or if string just fits buffer, we will
still write beyond the buffer as we append a '\0' byte at the end.

Will be nice to pass size of input buffer to split_fs_names() and
put enough checks in place so such buffer overrun possibilities
do not occur.

Hence this patch adds "size" parameter to split_fs_names() and makes
sure we do not access memory beyond size. If input string "names"
is larger than passed in buffer, input string will be truncated to
fit in buffer.

Reported-by: xu xin <xu.xin16@xxxxxxxxxx>
Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
---
init/do_mounts.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)

Index: redhat-linux/init/do_mounts.c
===================================================================
--- redhat-linux.orig/init/do_mounts.c 2021-09-15 08:46:33.801689806 -0400
+++ redhat-linux/init/do_mounts.c 2021-09-15 09:52:09.884449718 -0400
@@ -338,19 +338,20 @@ __setup("rootflags=", root_data_setup);
__setup("rootfstype=", fs_names_setup);
__setup("rootdelay=", root_delay_setup);

-static int __init split_fs_names(char *page, char *names)
+static int __init split_fs_names(char *page, size_t size, char *names)
{
int count = 0;
- char *p = page;
+ char *p = page, *end = page + size - 1;
+
+ strncpy(p, root_fs_names, size);
+ *end = '\0';

- strcpy(p, root_fs_names);
while (*p++) {
if (p[-1] == ',')
p[-1] = '\0';
}
- *p = '\0';

- for (p = page; *p; p += strlen(p)+1)
+ for (p = page; p < end && *p; p += strlen(p)+1)
count++;

return count;
@@ -404,7 +405,7 @@ void __init mount_block_root(char *name,
scnprintf(b, BDEVNAME_SIZE, "unknown-block(%u,%u)",
MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
if (root_fs_names)
- num_fs = split_fs_names(fs_names, root_fs_names);
+ num_fs = split_fs_names(fs_names, PAGE_SIZE, root_fs_names);
else
num_fs = list_bdev_fs_names(fs_names, PAGE_SIZE);
retry:
@@ -543,7 +544,7 @@ static int __init mount_nodev_root(void)
fs_names = (void *)__get_free_page(GFP_KERNEL);
if (!fs_names)
return -EINVAL;
- num_fs = split_fs_names(fs_names, root_fs_names);
+ num_fs = split_fs_names(fs_names, PAGE_SIZE, root_fs_names);

for (i = 0, fstype = fs_names; i < num_fs;
i++, fstype += strlen(fstype) + 1) {