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.
