Skip to content

Commit

Permalink
init/do_mounts.c: Harden split_fs_names() against buffer overflow
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Vivek Goyal <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Al Viro <[email protected]>
  • Loading branch information
rhvgoyal authored and Al Viro committed Sep 20, 2021
1 parent e4e737b commit b51593c
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions init/do_mounts.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit b51593c

Please sign in to comment.