Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1636047 > unrolled thread

[PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

Back to article view | Back to linux.kernel

Contents

  [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Rob Landley <rob@landley.net> - 2017-05-04 23:10 +0200
    Re: [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Kees Cook <keescook@chromium.org> - 2017-05-05 01:40 +0200
    Re: [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Andrew Morton <akpm@linux-foundation.org> - 2017-05-09 23:40 +0200
      Re: [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Rob Landley <rob@landley.net> - 2017-05-11 19:10 +0200
      [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Rob Landley <rob@landley.net> - 2017-05-11 19:10 +0200
        Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Rob Landley <rob@landley.net> - 2017-05-14 23:40 +0200
          Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Michael Ellerman <mpe@ellerman.id.au> - 2017-05-17 06:00 +0200
            Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT Rob Landley <rob@landley.net> - 2017-05-17 23:20 +0200

#1636047 — [PATCH] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

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

[toc] | [next] | [standalone]

#1636086

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

[toc] | [prev] | [next] | [standalone]

#1638410

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?

[toc] | [prev] | [next] | [standalone]

#1639900

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

[toc] | [prev] | [next] | [standalone]

#1639903 — [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

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

[toc] | [prev] | [next] | [standalone]

#1641223 — Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

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

[toc] | [prev] | [next] | [standalone]

#1642941 — Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

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

[toc] | [prev] | [next] | [standalone]

#1643636 — Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web