linux-kernel - Re: [PATCH RESEND] init/initramfs.c: make initramfs support pivot

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210521155020.GW4332@42.do-not-panic.com>
Date:   Fri, 21 May 2021 15:50:20 +0000
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Menglong Dong <menglong8.dong@...il.com>
Cc:     Jan Kara <jack@...e.cz>, Jens Axboe <axboe@...nel.dk>,
        hare@...e.de, gregkh@...uxfoundation.org, tj@...nel.org,
        Menglong Dong <dong.menglong@....com.cn>, song@...nel.org,
        neilb@...e.de, Andrew Morton <akpm@...ux-foundation.org>,
        wangkefeng.wang@...wei.com, f.fainelli@...il.com, arnd@...db.de,
        Barret Rhoden <brho@...gle.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        mhiramat@...nel.org, Steven Rostedt <rostedt@...dmis.org>,
        Kees Cook <keescook@...omium.org>, vbabka@...e.cz,
        Alexander Potapenko <glider@...gle.com>, pmladek@...e.com,
        Chris Down <chris@...isdown.name>, ebiederm@...ssion.com,
        jojing64@...il.com, LKML <linux-kernel@...r.kernel.org>,
        palmerdabbelt@...gle.com, linux-fsdevel@...r.kernel.org,
        Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH RESEND] init/initramfs.c: make initramfs support
 pivot_root

On Fri, May 21, 2021 at 08:41:55AM +0800, Menglong Dong wrote:
> Hello!
> 
> Thanks for your reply!
> 
> On Fri, May 21, 2021 at 5:41 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
> >
> > Can't docker instead allow to create containers prior to creating
> > your local docker network namespace? Not that its a great solution,
> > but just worth noting.
> >
> 
> That's a solution, but I don't think it is feasible. Users may create many
> containers, and you can't make docker create all the containers first
> and create network namespace later, as you don't know if there are any
> containers to create later.

It doesn't seem impossible, but worth noting why inside the commit log
this was not a preferred option.

> > >  struct file_system_type rootfs_fs_type = {
> > >       .name           = "rootfs",
> > > -     .init_fs_context = rootfs_init_fs_context,
> > > +     .init_fs_context = ramfs_init_fs_context,
> >
> > Why is this always static now? Why is that its correct
> > now for init_mount_tree() always to use the ramfs context?
> 
> Because the root mount in init_mount_tree() is not used as rootfs any more.

We still have:

start_kernel() --> vfs_caches_init() --> mnt_init() --> 

mnt_init()
{
	...
	shmem_init();                                                           
	init_rootfs();                                                          
	init_mount_tree(); 
}

You've now modified init_rootfs() to essentially just set the new user_root,
and that's it. But we stil call init_mount_tree() even if we did set the
rootfs to use tmpfs.

> In do_populate_ro
> tmpfs, and that's the real rootfs for initramfs. And I call this root
> as 'user_root',
> because it is created for user space.
> 
> int __init mount_user_root(void)
> {
>        return do_mount_root(user_root->dev_name,
>                             user_root->fs_name,
>                             root_mountflags,
>                             root_mount_data);
>  }
> 
> In other words, I moved the realization of 'rootfs_fs_type' here to
> do_populate_rootfs(), and fixed this 'rootfs_fs_type' with
> ramfs_init_fs_context, as it is a fake root now.

do_populate_rootfs() is called from populate_rootfs() and that in turn
is a:

rootfs_initcall(populate_rootfs);

In fact the latest changes have made this to schedule asynchronously as
well. And so indeed, init_mount_tree() always kicks off first. So its
still unclear to me why the first mount now always has a fs context of
ramfs_init_fs_context, even if we did not care for a ramdisk.

Are you suggesting it can be arbitrary now?

> Now, the rootfs that user space used is separated with the init_task,
> and that's exactly what a block root file system does.

The secondary effort is a bit clearer, its the earlier part that is not
so clear yet to me at least.

Regardless, to help make the changes easier to review, I wonder if it
makes sense to split up your work into a few patches. First do what you
have done for init_rootfs() and the structure you added to replace the
is_tmpfs bool, and let initialization use it and the context. And then
as a second patch introduce the second mount effort.

  Luis