summaryrefslogtreecommitdiffstats
path: root/init
diff options
context:
space:
mode:
authorVivek Goyal <vgoyal@redhat.com>2021-09-17 09:13:23 -0400
committerAl Viro <viro@zeniv.linux.org.uk>2021-09-19 22:24:49 -0400
commitb51593c4cd739dff7fc40bbed368572d98b19ae8 (patch)
tree434f93e808826013066d174613e5efcffe1740b0 /init
parente4e737bb5c170df6135a127739a9e6148ee3da82 (diff)
downloadlinux-b51593c4cd739dff7fc40bbed368572d98b19ae8.tar.bz2
init/do_mounts.c: Harden split_fs_names() against buffer overflow
split_fs_names() currently takes comma separate 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. Pass size of input buffer to split_fs_names() and put enough checks in place so such buffer overrun possibilities do not occur. This patch does few things. - Add a parameter "size" to split_fs_names(). This specifies size of input buffer. - Use strlcpy() (instead of strcpy()) so that we can't go beyond buffer size. If input string "names" is larger than passed in buffer, input string will be truncated to fit in buffer. - Stop appending extra '\0' character at the end and avoid one possibility of going beyond the input buffer size. - Do not use extra loop to count number of strings. - Previously if one passed "rootfstype=foo,,bar", split_fs_names() will return only 1 string "foo" (and "bar" will be truncated due to extra ,). After this patch, now split_fs_names() will return 3 strings ("foo", zero-sized-string, and "bar"). Callers of split_fs_names() have been modified to check for zero sized string and skip to next one. Reported-by: xu xin <xu.xin16@zte.com.cn> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> Reviewed-by: Jan Kara <jack@suse.cz> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'init')
-rw-r--r--init/do_mounts.c27
1 files changed, 16 insertions, 11 deletions
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 2ed30ff6c906..9207bde9ca3f 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -338,20 +338,19 @@ __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)
+/* This can return zero length strings. Caller should check */
+static int __init split_fs_names(char *page, size_t size, char *names)
{
- int count = 0;
+ int count = 1;
char *p = page;
- strcpy(p, root_fs_names);
+ strlcpy(p, root_fs_names, size);
while (*p++) {
- if (p[-1] == ',')
+ if (p[-1] == ',') {
p[-1] = '\0';
+ count++;
+ }
}
- *p = '\0';
-
- for (p = page; *p; p += strlen(p)+1)
- count++;
return count;
}
@@ -404,12 +403,16 @@ void __init mount_block_root(char *name, int flags)
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:
for (i = 0, p = fs_names; i < num_fs; i++, p += strlen(p)+1) {
- int err = do_mount_root(name, p, flags, root_mount_data);
+ int err;
+
+ if (!*p)
+ continue;
+ err = do_mount_root(name, p, flags, root_mount_data);
switch (err) {
case 0:
goto out;
@@ -543,10 +546,12 @@ 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) {
+ if (!*fstype)
+ continue;
if (!fs_is_nodev(fstype))
continue;
err = do_mount_root(root_device_name, fstype, root_mountflags,