[PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Rob Landley]

From: Rob Landley <rob@landley.net>

Make initramfs honor CONFIG_DEVTMPFS_MOUNT, and move
/dev/console open after devtmpfs mount.

Signed-off-by: Rob Landley <rob@landley.net>
---

 init/main.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/init/main.c b/init/main.c
index 2858be7..71ed0d7 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1016,12 +1016,6 @@ static noinline void __init kernel_init_freeable(void)
 
 	do_basic_setup();
 
-	/* Open the /dev/console on the rootfs, this should never fail */
-	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
-		pr_err("Warning: unable to open an initial console.\n");
-
-	(void) sys_dup(0);
-	(void) sys_dup(0);
 	/*
 	 * check if there is an early userspace init.  If yes, let it do all
 	 * the work
@@ -1033,8 +1027,17 @@ static noinline void __init kernel_init_freeable(void)
 	if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
 		ramdisk_execute_command = NULL;
 		prepare_namespace();
+	} else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
+		sys_mkdir("/dev", 0755);
+		sys_mount("dev", "dev", "devtmpfs", MS_SILENT, NULL);
 	}
 
+	/* Open the /dev/console on the rootfs, this should never fail */
+	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
+		pr_err("Warning: unable to open an initial console.\n");
+	(void) sys_dup(0);
+	(void) sys_dup(0);
+
 	/*
 	 * Ok, we have completed the initial bootup, and
 	 * we're essentially up and running. Get rid of the

Re: [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Kees Cook]

On Thu, May 4, 2017 at 2:09 PM, Rob Landley <rob@landley.net> wrote:
> From: Rob Landley <rob@landley.net>
>
> Make initramfs honor CONFIG_DEVTMPFS_MOUNT, and move
> /dev/console open after devtmpfs mount.
>
> Signed-off-by: Rob Landley <rob@landley.net>

Seems sensible.

Reviewed-by: Kees Cook <keescook@chromium.org>


-Kees

> ---
>
>  init/main.c |   15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 2858be7..71ed0d7 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1016,12 +1016,6 @@ static noinline void __init kernel_init_freeable(void)
>
>         do_basic_setup();
>
> -       /* Open the /dev/console on the rootfs, this should never fail */
> -       if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
> -               pr_err("Warning: unable to open an initial console.\n");
> -
> -       (void) sys_dup(0);
> -       (void) sys_dup(0);
>         /*
>          * check if there is an early userspace init.  If yes, let it do all
>          * the work
> @@ -1033,8 +1027,17 @@ static noinline void __init kernel_init_freeable(void)
>         if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
>                 ramdisk_execute_command = NULL;
>                 prepare_namespace();
> +       } else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
> +               sys_mkdir("/dev", 0755);
> +               sys_mount("dev", "dev", "devtmpfs", MS_SILENT, NULL);
>         }
>
> +       /* Open the /dev/console on the rootfs, this should never fail */
> +       if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
> +               pr_err("Warning: unable to open an initial console.\n");
> +       (void) sys_dup(0);
> +       (void) sys_dup(0);
> +
>         /*
>          * Ok, we have completed the initial bootup, and
>          * we're essentially up and running. Get rid of the



-- 
Kees Cook
Pixel Security

Re: [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Andrew Morton]

On Thu, 4 May 2017 16:09:06 -0500 Rob Landley <rob@landley.net> wrote:

> From: Rob Landley <rob@landley.net>
> 
> Make initramfs honor CONFIG_DEVTMPFS_MOUNT, and move
> /dev/console open after devtmpfs mount.


Could we please see complete description of the runtime effects of this
change?  How does it affect users?  How does it benefit users?

The DEVTMPFS_MOUNT Kconfig help (drivers/base/Kconfig) says:

	  This option does not affect initramfs based booting, here
	  the devtmpfs filesystem always needs to be mounted manually
	  after the rootfs is mounted.

which seems to no longer be correct?

Re: [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Rob Landley]

On 05/09/2017 04:31 PM, Andrew Morton wrote:
> On Thu, 4 May 2017 16:09:06 -0500 Rob Landley <rob@landley.net> wrote:
> 
>> From: Rob Landley <rob@landley.net>
>>
>> Make initramfs honor CONFIG_DEVTMPFS_MOUNT, and move
>> /dev/console open after devtmpfs mount.
> 
> 
> Could we please see complete description of the runtime effects of this
> change?  How does it affect users?  How does it benefit users?

It makes the behavior consistent. If you're going to have the config
symbol anyway, why is initramfs a second class citizen?

That said, I was fixing a specific bug when I started the patch: when
you statically link in an initramfs by pointing the kernel build at a
directory (so it makes its own cpio archive from that), if you're not
running the build as root you can't create dev/console in there and
there's no obvious way to add nodes (like you can editing the
gen_initramfs_list) output.

This means there's no /dev/console when init gets launched, so PID 1's
stdin/stdout/stderr go nowhere, and until your init script can open its
own and redirect you get no output if something goes wrong, so debugging
is fiddly and there's a hole where output gets lost. Userspace can't
close that hole.

When making the patch I did a version that mounted /proc /sys and
/dev/pts too, so rdinit=/bin/sh had pretty much its full environment
without an init script just like the DEVTMPFS_MOUNT option's help text
implied... but that seemed unlikely to be accepted. The console gap is a
problem userspace can't fix, the rest userspace can, so I did the
minimal thing.

> The DEVTMPFS_MOUNT Kconfig help (drivers/base/Kconfig) says:
> 
> 	  This option does not affect initramfs based booting, here
> 	  the devtmpfs filesystem always needs to be mounted manually
> 	  after the rootfs is mounted.
> 
> which seems to no longer be correct?

Ah, sorry. I rewrote the help text and didn't include that file in the
diff. And rechecking I see the override part wasn't implemented by my
patch, I'll send a new one.

Rob

[PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Rob Landley]

From: Rob Landley <rob@landley.net>

Make initramfs honor CONFIG_DEVTMPFS_MOUNT, move /dev/console
open after devtmpfs mount, and update help text.

Signed-off-by: Rob Landley <rob@landley.net>
---

 drivers/base/Kconfig |   14 ++++----------
 init/main.c          |   15 +++++++++------
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index d718ae4..74779ee 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -48,16 +48,10 @@ config DEVTMPFS_MOUNT
 	bool "Automount devtmpfs at /dev, after the kernel mounted the rootfs"
 	depends on DEVTMPFS
 	help
-	  This will instruct the kernel to automatically mount the
-	  devtmpfs filesystem at /dev, directly after the kernel has
-	  mounted the root filesystem. The behavior can be overridden
-	  with the commandline parameter: devtmpfs.mount=0|1.
-	  This option does not affect initramfs based booting, here
-	  the devtmpfs filesystem always needs to be mounted manually
-	  after the rootfs is mounted.
-	  With this option enabled, it allows to bring up a system in
-	  rescue mode with init=/bin/sh, even when the /dev directory
-	  on the rootfs is completely empty.
+	  Automatically mount devtmpfs at /dev on the root filesystem, which
+	  lets the system come up in rescue mode with [rd]init=/bin/sh.
+	  Override with devtmpfs.mount=0 on the commandline. Initramfs can
+	  create a /dev dir as needed, other rootfs needs the mount point.
 
 config STANDALONE
 	bool "Select only drivers that don't need compile-time external firmware"
diff --git a/init/main.c b/init/main.c
index f866510..9ec09ff 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1038,12 +1038,6 @@ static noinline void __init kernel_init_freeable(void)
 
 	do_basic_setup();
 
-	/* Open the /dev/console on the rootfs, this should never fail */
-	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
-		pr_err("Warning: unable to open an initial console.\n");
-
-	(void) sys_dup(0);
-	(void) sys_dup(0);
 	/*
 	 * check if there is an early userspace init.  If yes, let it do all
 	 * the work
@@ -1055,8 +1049,17 @@ static noinline void __init kernel_init_freeable(void)
 	if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
 		ramdisk_execute_command = NULL;
 		prepare_namespace();
+	} else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
+		sys_mkdir("/dev", 0755);
+		devtmpfs_mount("/dev");
 	}
 
+	/* Open the /dev/console on the rootfs, this should never fail */
+	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
+		pr_err("Warning: unable to open an initial console.\n");
+	(void) sys_dup(0);
+	(void) sys_dup(0);
+
 	/*
 	 * Ok, we have completed the initial bootup, and
 	 * we're essentially up and running. Get rid of the

Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Rob Landley]

Andrew asked for "a more complete changelog" and I've had
a reply window open for _days_ trying to figure out
what he wants. Maybe it's in the following somewhere...

Otherwise the same v2 patch.

From: Rob Landley <rob@landley.net>

Make initramfs honor CONFIG_DEVTMPFS_MOUNT (fixing commit
2b2af54a5bb6 which didn't bother), move /dev/console
open after devtmpfs mount, and update help text.

Commit 456eeabab849 in 2005 made gen_initramfs_list (when run
with no arguments) spit out an 'example' config creating /dev
and /dev/console. The kernel accidentally(?) included this
for many years when you didn't specify initramfs contents,
and of course grew dependencies on this /dev/console node
in the (often hidden) initramfs. Commit c33df4eaaf41 in 2007
explicitly preserved this dependency. Commit 2bd3a997befc in
2010 claimed it "removes the occasionally problematic assumption
that /dev/console exists from the boot code" but actually just
moved it later.

But nobody never tested statically linking an initramfs.
If you point CONFIG_INITRAMFS_SOURCE at a directory
running the build as a normal user you _don't_ get a
/dev/console (because you can't create it without being
root, and can't use the existing one out of /dev unless
you create your own initramfs list file), in which case init
runs with stdin/stdout/stderr closed and you get no output.

Eric's test case for his 2010 commit referenced above was:

  With this patch I was able to throw busybox on my /boot partition
  (which has no /dev directory) and boot into userspace without
  problems.

But it didn't work pointing CONFIG_INITRAMFS_SOURCE at a
directory of the same files. This provides the "automatically
mounting devtmpfs on /dev" workaround the earlier commit was
trying to avoid.

Signed-off-by: Rob Landley <rob@landley.net>
---

 drivers/base/Kconfig |   14 ++++----------
 init/main.c          |   15 +++++++++------
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index d718ae4..74779ee 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -48,16 +48,10 @@ config DEVTMPFS_MOUNT
 	bool "Automount devtmpfs at /dev, after the kernel mounted the rootfs"
 	depends on DEVTMPFS
 	help
-	  This will instruct the kernel to automatically mount the
-	  devtmpfs filesystem at /dev, directly after the kernel has
-	  mounted the root filesystem. The behavior can be overridden
-	  with the commandline parameter: devtmpfs.mount=0|1.
-	  This option does not affect initramfs based booting, here
-	  the devtmpfs filesystem always needs to be mounted manually
-	  after the rootfs is mounted.
-	  With this option enabled, it allows to bring up a system in
-	  rescue mode with init=/bin/sh, even when the /dev directory
-	  on the rootfs is completely empty.
+	  Automatically mount devtmpfs at /dev on the root filesystem, which
+	  lets the system to come up in rescue mode with [rd]init=/bin/sh.
+	  Override with devtmpfs.mount=0 on the commandline. Initramfs can
+	  create a /dev dir as needed, other rootfs needs the mount point.
 
 config STANDALONE
 	bool "Select only drivers that don't need compile-time external firmware"
diff --git a/init/main.c b/init/main.c
index f866510..9ec09ff 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1038,12 +1038,6 @@ static noinline void __init kernel_init_freeable(void)
 
 	do_basic_setup();
 
-	/* Open the /dev/console on the rootfs, this should never fail */
-	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
-		pr_err("Warning: unable to open an initial console.\n");
-
-	(void) sys_dup(0);
-	(void) sys_dup(0);
 	/*
 	 * check if there is an early userspace init.  If yes, let it do all
 	 * the work
@@ -1055,8 +1049,17 @@ static noinline void __init kernel_init_freeable(void)
 	if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
 		ramdisk_execute_command = NULL;
 		prepare_namespace();
+	} else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
+		sys_mkdir("/dev", 0755);
+		devtmpfs_mount("/dev");
 	}
 
+	/* Open the /dev/console on the rootfs, this should never fail */
+	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
+		pr_err("Warning: unable to open an initial console.\n");
+	(void) sys_dup(0);
+	(void) sys_dup(0);
+
 	/*
 	 * Ok, we have completed the initial bootup, and
 	 * we're essentially up and running. Get rid of the

Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Michael Ellerman]

Rob Landley <rob@landley.net> writes:

> diff --git a/init/main.c b/init/main.c
> index f866510..9ec09ff 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1055,8 +1049,17 @@ static noinline void __init kernel_init_freeable(void)
>  	if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
>  		ramdisk_execute_command = NULL;
>  		prepare_namespace();
> +	} else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
> +		sys_mkdir("/dev", 0755);
> +		devtmpfs_mount("/dev");
>  	}
>  
> +	/* Open the /dev/console on the rootfs, this should never fail */
> +	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)

Sorry to pile on, but while you're moving it do you want to update this
fairly misleading comment.

It definitely can fail, eg. if /dev/console doesn't exist, or if no
console driver is registered.

cheers

Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

[Rob Landley]

On 05/16/2017 10:58 PM, Michael Ellerman wrote:
> Rob Landley <rob@landley.net> writes:
> 
>> diff --git a/init/main.c b/init/main.c
>> index f866510..9ec09ff 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -1055,8 +1049,17 @@ static noinline void __init kernel_init_freeable(void)
>>  	if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
>>  		ramdisk_execute_command = NULL;
>>  		prepare_namespace();
>> +	} else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
>> +		sys_mkdir("/dev", 0755);
>> +		devtmpfs_mount("/dev");
>>  	}
>>  
>> +	/* Open the /dev/console on the rootfs, this should never fail */
>> +	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
> 
> Sorry to pile on,

The correct phrase is "bikeshed". (It's a verb now.)

> but while you're moving it do you want

_I_ don't, no. I intentionally moved it unmodified. If you want to
submit a patch on top of mine, be my guest.

> to update this fairly misleading comment.

Define "should". (I'll get the popcorn.)

> It definitely can fail, eg. if /dev/console doesn't exist, or if no
> console driver is registered.

/dev/console not existing in an initramfs created by pointing
CONFIG_INITRAMFS_SOURCE at a directory created by a normal user was
pretty much my initial motivation for poking at this area, yes.

That said, /dev/console should always exist. My patch was just finding a
different way for it to exist so the condition was satisfied. Meaning
the comment isn't exactly _wrong_, just really terse. Feel free to
submit a patch rephrasing it.

Rob