linux-kernel - Re: [PATCH v3] init/do_mounts.c: Harden split_fs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YUfwmdWTylRidtkq@zeniv-ca.linux.org.uk>
Date:   Mon, 20 Sep 2021 02:23:21 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Vivek Goyal <vgoyal@...hat.com>
Cc:     Christoph Hellwig <hch@...radead.org>, Jan Kara <jack@...e.cz>,
        Linux fsdevel mailing list <linux-fsdevel@...r.kernel.org>,
        linux kernel mailing list <linux-kernel@...r.kernel.org>,
        xu.xin16@....com.cn, christian.brauner@...ntu.com
Subject: Re: [PATCH v3] init/do_mounts.c: Harden split_fs_names() against
 buffer overflow

On Fri, Sep 17, 2021 at 09:13:23AM -0400, Vivek Goyal wrote:

> --- redhat-linux.orig/init/do_mounts.c	2021-09-15 08:46:33.801689806 -0400
> +++ redhat-linux/init/do_mounts.c	2021-09-17 08:44:40.781430167 -0400
> @@ -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,
>  	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,


Applied.