Re: [PATCH 1/2] init: ensure that /dev/console is (nearly) always available in initramfs

[Askar Safin]

David Disseldorp <ddiss@xxxxxxx>:
> I'd prefer not to go down this path:
> - I think it's reasonable to expect that users who override the default
>   internal initramfs know what they're doing WRT /dev/console creation.
> - initramfs can be made up of concatenated cpio archives, so tools which
>   insist on using GNU cpio and run into mknod EPERM issues could append
>   the nodes via gen_init_cpio, while continuing to use GNU cpio for
>   everything else.

This cannot be done in proper way.

Let's assume we want to build *builtin* initramfs using GNU cpio and
then concatenate to it an archive made by gen_init_cpio.

Then we want CONFIG_INITRAMFS_SOURCE to accept already existing cpio
archive AND file in gen_init_cpio format. But, according to
CONFIG_INITRAMFS_SOURCE docs in usr/Kconfig, this is not possible
(I didn't check whether this is true, I just have read the docs.)

This means that we should do this:

1. Generate an archive by invoking gen_init_cpio and concatenate it to
our preexisting archive
2. Create kernel config, while specifying resulting archive in
CONFIG_INITRAMFS_SOURCE
3. Build the kernel

Unfortunately, this will not work, because to invoke gen_init_cpio you
should build it first. And you cannot build it if there is no config
(I checked this).

So, we should do this:

1. Create kernel config, while specifying an archive in
CONFIG_INITRAMFS_SOURCE, which *DOES NOT EXISTS YET*
2. Build gen_init_cpio by invoking "make usr/gen_init_cpio"
3. Create an archive by invoking gen_init_cpio and concatenate it to
our preexisting archive. Put resulting archive to the path specified in
CONFIG_INITRAMFS_SOURCE
4. Build the kernel

Unfortunately, this will not work, either, because command
"make usr/gen_init_cpio" doesn't work in clean kernel tree even if
config exists (I checked this).

This means that the only remaining way is this:

1. Create *fake* kernel config
2. Build whole kernel (!!!)
3. Create an archive by invoking gen_init_cpio and concatenate it to
our preexisting archive
4. Create config, this time for real. Specify archive created in previous
step as CONFIG_INITRAMFS_SOURCE
5. Build the kernel, this time for real

I hope you agree that this is totally insane.

So, there is no proper way to create builtin initramfs by concatenating
archives created by GNU cpio and gen_init_cpio.

I think this is a bug in kbuild, and it probably needs to be fixed.

But I think that my patchset is a better approach. My patchset simply
ensures that /dev/console and /dev/null are always available, no matter
what. And thus my patchset side steps the whole issue.

Here are additional arguments in favor of my patchset.

* The kernel itself relies on presence of /dev/console in console_on_rootfs.
There is even comment there that currently says that opening of /dev/console
should never fail. (The comment is wrong, and this patchset fixes it.) So, if you
happen to supply builtin initramfs without /dev/console, then you will
build kernel, which violates its own assumptions. This will not be
detected until we actually try to boot the kernel. And even then it will
be hard to understand what exactly went wrong.

Why should we allow this failure mode? Let's instead ensure that this will
never happen. I. e. let's make sure that the kernel's asssumptions are always
true!

* My patchset makes the kernel not more complex, but more simple!
(28 insertions(+), 33 deletions(-).) Moreover, my patchset makes it
simpler not only in LoC sense, but in conceptual sense, too!
Currently codes in usr/default_cpio_list and init/noinitramfs.c are
very similar. It is possible that they will be out of sync in
some point of future.

By the way, noticing that they are out of sync is *very* hard. Consider
this scenario: usr/default_cpio_list contains this line:

nod /dev/null 0666 0 0 c 1 3

and init/noinitramfs.c contains this line:

init_mknod("/dev/null", S_IFCHR | 0666,
                        new_encode_dev(MKDEV(1, 3)));

Are these lines equivalent? You may think they are, but they are not.

init_mknod function above in fact creates node with different rights
due to umask of kernel thread. (And thus unprivileged users will be unable
to write to /dev/null...)

My patchset merges both codes into a single helper, thus making sure they
will never be out of sync. And thus my patchset reduces complexity of
the kernel.

* Currently it is okay not to put /dev/console to external initramfs.
But it is not okay not to put it to builtin initramfs. Why creating
builtin initramfs is harder than creating external initramfs? Why we make
lifes of developers in one of these cases harder without any reason?

Consider this scenario: somebody built kernel and external initramfs.
Everything works. Now they make this initramfs to be builtin.
Of course, they expect that everything will continue to work as before.
But it doesn't work. PID 1 doesn't print anything to console,
and you cannot even debug this, because it doesn't print anything to
console and you cannot see your debug printfs!

This clearly violates principle of least surprise.

-- 
Askar Safin