linux-kernel - Re: [PATCH v2 0/2] Add support to boot virtiofs and 9pfs as rootfs

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YMtbPDW+T5Z1uBZt@infradead.org>
Date:   Thu, 17 Jun 2021 15:25:00 +0100
From:   Christoph Hellwig <hch@...radead.org>
To:     Vivek Goyal <vgoyal@...hat.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        virtio-fs@...hat.com, miklos@...redi.hu, stefanha@...hat.com,
        dgilbert@...hat.com, viro@...iv.linux.org.uk, dhowells@...hat.com,
        richard.weinberger@...il.com, asmadeus@...ewreck.org,
        v9fs-developer@...ts.sourceforge.net
Subject: Re: [PATCH v2 0/2] Add support to boot virtiofs and 9pfs as rootfs

On Thu, Jun 17, 2021 at 09:30:52AM -0400, Vivek Goyal wrote:
> > +static int __init mount_nodev_root(void)
> > +{
> > +	struct file_system_type *fs = get_fs_type(root_fs_names);
> 
> get_fs_type() assumes root_fs_names is not null. So if I pass
> "root=myfs rw", it crashes with null pointer dereference.

Ok, I'll need to fix that.

> > +	int err = -ENODEV;
> > +
> > +	if (!fs)
> > +		goto out;
> > +	if (fs->fs_flags & FS_REQUIRES_DEV)
> > +		goto out_put_filesystem;
> > +
> > +	fs_names = (void *)__get_free_page(GFP_KERNEL);
> > +	if (!fs_names)
> > +		goto out_put_filesystem;
> > +	get_fs_names(fs_names);
> 
> I am wondering what use case we are trying to address by calling
> get_fs_names() and trying do_mount_root() on all filesystems
> returned by get_fs_names(). I am assuming following use cases
> you have in mind.
> 
> A. User passes a single filesystem in rootfstype.
>    
>    root=myfs rootfstype=virtiofs rw
> 
> B. User passes multiple filesystems in rootfstype and kernel tries all
>    of them one after the other
> 
>    root=myfs, rootfstype=9p,virtiofs rw
> 
> C. User does not pass a filesystem type at all. And kernel will get a
>    list of in-built filesystems and will try these one after the other.
> 
>    root=myfs rw
> 
> If that's the thought, will it make sense to call get_fs_names() first
> and then inside the for loop call get_fs_type() and try mounting
> only if FS_REQUIRES_DEV is not set, otherwise skip and move onto th
> next filesystem in the list (fs_names).

I thought of A and B.  I did not think at all of C and think it is
a rather bad idea.  I'll revisit the patch to avoid C and will resend it
as a formal patch.