All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] init: Add strictinit to disable init= fallbacks
@ 2014-09-26 19:13 Andy Lutomirski
  2014-09-26 19:23 ` Chuck Ebbert
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Lutomirski @ 2014-09-26 19:13 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel
  Cc: Randy Dunlap, Shuah Khan, Rusty Russell, Andy Lutomirski

If a user puts init=/whatever on the command line and /whatever
can't be run, then the kernel will try a few default options before
giving up.  If init=/whatever came from a bootloader prompt, then
this probably makes sense.  On the other hand, if it comes from a
script (e.g. a tool like virtme or perhaps a future kselftest
script), then the fallbacks are likely to exist, but they'll do the
wrong thing.  For example, they might unexpectedly invoke systemd.

This adds a new option called strictinit.  If init= and strictinit
are both set, and the init= binary is not executable, then the
kernel will panic immediately.  If strictinit is set but init= is
not set, then strictinit will have no effect, because the only real
alternative would be to panic regardless of the contents of the root
fs.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 Documentation/kernel-parameters.txt |  8 ++++++++
 init/main.c                         | 16 ++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 10d51c2f10d7..1576273edce6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3236,6 +3236,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	stifb=		[HW]
 			Format: bpp:<bpp1>[:<bpp2>[:<bpp3>...]]
 
+	strictinit	[KNL,BOOT]
+			Normally, if the kernel can't find the init binary
+			specified by rdinit= and/or init=, then it will
+			try several fallbacks.  If strictinit is set
+			and the value specified by init= does not work,
+			then the kernel will panic instead.
+			This option makes no sense if init= is not specified.
+
 	sunrpc.min_resvport=
 	sunrpc.max_resvport=
 			[NFS,SUNRPC]
diff --git a/init/main.c b/init/main.c
index bb1aed928f21..2ae0f2776155 100644
--- a/init/main.c
+++ b/init/main.c
@@ -131,6 +131,7 @@ static char *initcall_command_line;
 
 static char *execute_command;
 static char *ramdisk_execute_command;
+static bool strictinit;
 
 /*
  * Used to generate warnings if static_key manipulation functions are used
@@ -347,6 +348,13 @@ static int __init rdinit_setup(char *str)
 }
 __setup("rdinit=", rdinit_setup);
 
+static int __init strictinit_setup(char *str)
+{
+	strictinit = true;
+	return 1;
+}
+__setup("strictinit", strictinit_setup);
+
 #ifndef CONFIG_SMP
 static const unsigned int setup_max_cpus = NR_CPUS;
 #ifdef CONFIG_X86_LOCAL_APIC
@@ -960,8 +968,12 @@ static int __ref kernel_init(void *unused)
 		ret = run_init_process(execute_command);
 		if (!ret)
 			return 0;
-		pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
-			execute_command, ret);
+		if (strictinit)
+			panic("Requested init %s failed (error %d) and strictinit was set.",
+			      execute_command, ret);
+		else
+			pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
+			       execute_command, ret);
 	}
 	if (!try_to_run_init_process("/sbin/init") ||
 	    !try_to_run_init_process("/etc/init") ||
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] init: Add strictinit to disable init= fallbacks
  2014-09-26 19:13 [PATCH v3] init: Add strictinit to disable init= fallbacks Andy Lutomirski
@ 2014-09-26 19:23 ` Chuck Ebbert
  2014-09-26 19:30   ` Rob Landley
  2014-09-26 19:31   ` Andy Lutomirski
  0 siblings, 2 replies; 5+ messages in thread
From: Chuck Ebbert @ 2014-09-26 19:23 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Andrew Morton, linux-kernel, Randy Dunlap, Shuah Khan,
	Rusty Russell

On Fri, 26 Sep 2014 12:13:57 -0700
Andy Lutomirski <luto@amacapital.net> wrote:

> If a user puts init=/whatever on the command line and /whatever
> can't be run, then the kernel will try a few default options before
> giving up.  If init=/whatever came from a bootloader prompt, then
> this probably makes sense.  On the other hand, if it comes from a
> script (e.g. a tool like virtme or perhaps a future kselftest
> script), then the fallbacks are likely to exist, but they'll do the
> wrong thing.  For example, they might unexpectedly invoke systemd.
> 
> This adds a new option called strictinit.  If init= and strictinit
> are both set, and the init= binary is not executable, then the
> kernel will panic immediately.  If strictinit is set but init= is
> not set, then strictinit will have no effect, because the only real
> alternative would be to panic regardless of the contents of the root
> fs.
> 
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  Documentation/kernel-parameters.txt |  8 ++++++++
>  init/main.c                         | 16 ++++++++++++++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 10d51c2f10d7..1576273edce6 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3236,6 +3236,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  	stifb=		[HW]
>  			Format: bpp:<bpp1>[:<bpp2>[:<bpp3>...]]
>  
> +	strictinit	[KNL,BOOT]
> +			Normally, if the kernel can't find the init binary
> +			specified by rdinit= and/or init=, then it will
> +			try several fallbacks.  If strictinit is set
> +			and the value specified by init= does not work,
> +			then the kernel will panic instead.
> +			This option makes no sense if init= is not specified.
> +
>  	sunrpc.min_resvport=
>  	sunrpc.max_resvport=
>  			[NFS,SUNRPC]
> diff --git a/init/main.c b/init/main.c
> index bb1aed928f21..2ae0f2776155 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -131,6 +131,7 @@ static char *initcall_command_line;
>  
>  static char *execute_command;
>  static char *ramdisk_execute_command;
> +static bool strictinit;
>  
>  /*
>   * Used to generate warnings if static_key manipulation functions are used
> @@ -347,6 +348,13 @@ static int __init rdinit_setup(char *str)
>  }
>  __setup("rdinit=", rdinit_setup);
>  
> +static int __init strictinit_setup(char *str)
> +{
> +	strictinit = true;
> +	return 1;
> +}
> +__setup("strictinit", strictinit_setup);
> +
>  #ifndef CONFIG_SMP
>  static const unsigned int setup_max_cpus = NR_CPUS;
>  #ifdef CONFIG_X86_LOCAL_APIC
> @@ -960,8 +968,12 @@ static int __ref kernel_init(void *unused)
>  		ret = run_init_process(execute_command);
>  		if (!ret)
>  			return 0;
> -		pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
> -			execute_command, ret);
> +		if (strictinit)
> +			panic("Requested init %s failed (error %d) and strictinit was set.",
> +			      execute_command, ret);
> +		else
> +			pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
> +			       execute_command, ret);
>  	}
>  	if (!try_to_run_init_process("/sbin/init") ||
>  	    !try_to_run_init_process("/etc/init") ||

Can't you just make it use "init=foo,strict" instead?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] init: Add strictinit to disable init= fallbacks
  2014-09-26 19:23 ` Chuck Ebbert
@ 2014-09-26 19:30   ` Rob Landley
  2014-09-26 19:32     ` Andy Lutomirski
  2014-09-26 19:31   ` Andy Lutomirski
  1 sibling, 1 reply; 5+ messages in thread
From: Rob Landley @ 2014-09-26 19:30 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Andy Lutomirski, Andrew Morton, Kernel Mailing List, Randy Dunlap,
	Shuah Khan, Rusty Russell

On Fri, Sep 26, 2014 at 2:23 PM, Chuck Ebbert <cebbert.lkml@gmail.com> wrote:
> On Fri, 26 Sep 2014 12:13:57 -0700
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> If a user puts init=/whatever on the command line and /whatever
>> can't be run, then the kernel will try a few default options before
>> giving up.  If init=/whatever came from a bootloader prompt, then
>> this probably makes sense.  On the other hand, if it comes from a
>> script (e.g. a tool like virtme or perhaps a future kselftest
>> script), then the fallbacks are likely to exist, but they'll do the
>> wrong thing.  For example, they might unexpectedly invoke systemd.
>>
>> This adds a new option called strictinit.  If init= and strictinit
>> are both set, and the init= binary is not executable, then the
>> kernel will panic immediately.  If strictinit is set but init= is
>> not set, then strictinit will have no effect, because the only real
>> alternative would be to panic regardless of the contents of the root
>> fs.
>>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> ---
>>  Documentation/kernel-parameters.txt |  8 ++++++++
>>  init/main.c                         | 16 ++++++++++++++--
>>  2 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 10d51c2f10d7..1576273edce6 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -3236,6 +3236,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>       stifb=          [HW]
>>                       Format: bpp:<bpp1>[:<bpp2>[:<bpp3>...]]
>>
>> +     strictinit      [KNL,BOOT]
>> +                     Normally, if the kernel can't find the init binary
>> +                     specified by rdinit= and/or init=, then it will
>> +                     try several fallbacks.  If strictinit is set
>> +                     and the value specified by init= does not work,
>> +                     then the kernel will panic instead.
>> +                     This option makes no sense if init= is not specified.
>> +
>>       sunrpc.min_resvport=
>>       sunrpc.max_resvport=
>>                       [NFS,SUNRPC]
>> diff --git a/init/main.c b/init/main.c
>> index bb1aed928f21..2ae0f2776155 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -131,6 +131,7 @@ static char *initcall_command_line;
>>
>>  static char *execute_command;
>>  static char *ramdisk_execute_command;
>> +static bool strictinit;
>>
>>  /*
>>   * Used to generate warnings if static_key manipulation functions are used
>> @@ -347,6 +348,13 @@ static int __init rdinit_setup(char *str)
>>  }
>>  __setup("rdinit=", rdinit_setup);
>>
>> +static int __init strictinit_setup(char *str)
>> +{
>> +     strictinit = true;
>> +     return 1;
>> +}
>> +__setup("strictinit", strictinit_setup);
>> +
>>  #ifndef CONFIG_SMP
>>  static const unsigned int setup_max_cpus = NR_CPUS;
>>  #ifdef CONFIG_X86_LOCAL_APIC
>> @@ -960,8 +968,12 @@ static int __ref kernel_init(void *unused)
>>               ret = run_init_process(execute_command);
>>               if (!ret)
>>                       return 0;
>> -             pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
>> -                     execute_command, ret);
>> +             if (strictinit)
>> +                     panic("Requested init %s failed (error %d) and strictinit was set.",
>> +                           execute_command, ret);
>> +             else
>> +                     pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
>> +                            execute_command, ret);
>>       }
>>       if (!try_to_run_init_process("/sbin/init") ||
>>           !try_to_run_init_process("/etc/init") ||
>
> Can't you just make it use "init=foo,strict" instead?

Can't we just change the default behavior and add a
CONFIG_INIT_FALLBACK that defaults to "n" which you can switch on to
get the old behavior? (And then immediately deprecate it?)

If you're specifying an init, you probably want that init...

Rob

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] init: Add strictinit to disable init= fallbacks
  2014-09-26 19:23 ` Chuck Ebbert
  2014-09-26 19:30   ` Rob Landley
@ 2014-09-26 19:31   ` Andy Lutomirski
  1 sibling, 0 replies; 5+ messages in thread
From: Andy Lutomirski @ 2014-09-26 19:31 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Andrew Morton, linux-kernel@vger.kernel.org, Randy Dunlap,
	Shuah Khan, Rusty Russell

On Fri, Sep 26, 2014 at 12:23 PM, Chuck Ebbert <cebbert.lkml@gmail.com> wrote:
> On Fri, 26 Sep 2014 12:13:57 -0700
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> If a user puts init=/whatever on the command line and /whatever
>> can't be run, then the kernel will try a few default options before
>> giving up.  If init=/whatever came from a bootloader prompt, then
>> this probably makes sense.  On the other hand, if it comes from a
>> script (e.g. a tool like virtme or perhaps a future kselftest
>> script), then the fallbacks are likely to exist, but they'll do the
>> wrong thing.  For example, they might unexpectedly invoke systemd.
>>
>> This adds a new option called strictinit.  If init= and strictinit
>> are both set, and the init= binary is not executable, then the
>> kernel will panic immediately.  If strictinit is set but init= is
>> not set, then strictinit will have no effect, because the only real
>> alternative would be to panic regardless of the contents of the root
>> fs.
>>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> ---
>>  Documentation/kernel-parameters.txt |  8 ++++++++
>>  init/main.c                         | 16 ++++++++++++++--
>>  2 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 10d51c2f10d7..1576273edce6 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -3236,6 +3236,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>       stifb=          [HW]
>>                       Format: bpp:<bpp1>[:<bpp2>[:<bpp3>...]]
>>
>> +     strictinit      [KNL,BOOT]
>> +                     Normally, if the kernel can't find the init binary
>> +                     specified by rdinit= and/or init=, then it will
>> +                     try several fallbacks.  If strictinit is set
>> +                     and the value specified by init= does not work,
>> +                     then the kernel will panic instead.
>> +                     This option makes no sense if init= is not specified.
>> +
>>       sunrpc.min_resvport=
>>       sunrpc.max_resvport=
>>                       [NFS,SUNRPC]
>> diff --git a/init/main.c b/init/main.c
>> index bb1aed928f21..2ae0f2776155 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -131,6 +131,7 @@ static char *initcall_command_line;
>>
>>  static char *execute_command;
>>  static char *ramdisk_execute_command;
>> +static bool strictinit;
>>
>>  /*
>>   * Used to generate warnings if static_key manipulation functions are used
>> @@ -347,6 +348,13 @@ static int __init rdinit_setup(char *str)
>>  }
>>  __setup("rdinit=", rdinit_setup);
>>
>> +static int __init strictinit_setup(char *str)
>> +{
>> +     strictinit = true;
>> +     return 1;
>> +}
>> +__setup("strictinit", strictinit_setup);
>> +
>>  #ifndef CONFIG_SMP
>>  static const unsigned int setup_max_cpus = NR_CPUS;
>>  #ifdef CONFIG_X86_LOCAL_APIC
>> @@ -960,8 +968,12 @@ static int __ref kernel_init(void *unused)
>>               ret = run_init_process(execute_command);
>>               if (!ret)
>>                       return 0;
>> -             pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
>> -                     execute_command, ret);
>> +             if (strictinit)
>> +                     panic("Requested init %s failed (error %d) and strictinit was set.",
>> +                           execute_command, ret);
>> +             else
>> +                     pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
>> +                            execute_command, ret);
>>       }
>>       if (!try_to_run_init_process("/sbin/init") ||
>>           !try_to_run_init_process("/etc/init") ||
>
> Can't you just make it use "init=foo,strict" instead?

I don't think so.  Old kernels will wonder why "foo,strict" doesn't exist.

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] init: Add strictinit to disable init= fallbacks
  2014-09-26 19:30   ` Rob Landley
@ 2014-09-26 19:32     ` Andy Lutomirski
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Lutomirski @ 2014-09-26 19:32 UTC (permalink / raw)
  To: Rob Landley
  Cc: Chuck Ebbert, Andrew Morton, Kernel Mailing List, Randy Dunlap,
	Shuah Khan, Rusty Russell

On Fri, Sep 26, 2014 at 12:30 PM, Rob Landley <rob@landley.net> wrote:
> On Fri, Sep 26, 2014 at 2:23 PM, Chuck Ebbert <cebbert.lkml@gmail.com> wrote:
>> On Fri, 26 Sep 2014 12:13:57 -0700
>> Andy Lutomirski <luto@amacapital.net> wrote:
>>
>>> If a user puts init=/whatever on the command line and /whatever
>>> can't be run, then the kernel will try a few default options before
>>> giving up.  If init=/whatever came from a bootloader prompt, then
>>> this probably makes sense.  On the other hand, if it comes from a
>>> script (e.g. a tool like virtme or perhaps a future kselftest
>>> script), then the fallbacks are likely to exist, but they'll do the
>>> wrong thing.  For example, they might unexpectedly invoke systemd.
>>>
>>> This adds a new option called strictinit.  If init= and strictinit
>>> are both set, and the init= binary is not executable, then the
>>> kernel will panic immediately.  If strictinit is set but init= is
>>> not set, then strictinit will have no effect, because the only real
>>> alternative would be to panic regardless of the contents of the root
>>> fs.
>>>
>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>> ---
>>>  Documentation/kernel-parameters.txt |  8 ++++++++
>>>  init/main.c                         | 16 ++++++++++++++--
>>>  2 files changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>> index 10d51c2f10d7..1576273edce6 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -3236,6 +3236,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>       stifb=          [HW]
>>>                       Format: bpp:<bpp1>[:<bpp2>[:<bpp3>...]]
>>>
>>> +     strictinit      [KNL,BOOT]
>>> +                     Normally, if the kernel can't find the init binary
>>> +                     specified by rdinit= and/or init=, then it will
>>> +                     try several fallbacks.  If strictinit is set
>>> +                     and the value specified by init= does not work,
>>> +                     then the kernel will panic instead.
>>> +                     This option makes no sense if init= is not specified.
>>> +
>>>       sunrpc.min_resvport=
>>>       sunrpc.max_resvport=
>>>                       [NFS,SUNRPC]
>>> diff --git a/init/main.c b/init/main.c
>>> index bb1aed928f21..2ae0f2776155 100644
>>> --- a/init/main.c
>>> +++ b/init/main.c
>>> @@ -131,6 +131,7 @@ static char *initcall_command_line;
>>>
>>>  static char *execute_command;
>>>  static char *ramdisk_execute_command;
>>> +static bool strictinit;
>>>
>>>  /*
>>>   * Used to generate warnings if static_key manipulation functions are used
>>> @@ -347,6 +348,13 @@ static int __init rdinit_setup(char *str)
>>>  }
>>>  __setup("rdinit=", rdinit_setup);
>>>
>>> +static int __init strictinit_setup(char *str)
>>> +{
>>> +     strictinit = true;
>>> +     return 1;
>>> +}
>>> +__setup("strictinit", strictinit_setup);
>>> +
>>>  #ifndef CONFIG_SMP
>>>  static const unsigned int setup_max_cpus = NR_CPUS;
>>>  #ifdef CONFIG_X86_LOCAL_APIC
>>> @@ -960,8 +968,12 @@ static int __ref kernel_init(void *unused)
>>>               ret = run_init_process(execute_command);
>>>               if (!ret)
>>>                       return 0;
>>> -             pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
>>> -                     execute_command, ret);
>>> +             if (strictinit)
>>> +                     panic("Requested init %s failed (error %d) and strictinit was set.",
>>> +                           execute_command, ret);
>>> +             else
>>> +                     pr_err("Failed to execute %s (error %d).  Attempting defaults...\n",
>>> +                            execute_command, ret);
>>>       }
>>>       if (!try_to_run_init_process("/sbin/init") ||
>>>           !try_to_run_init_process("/etc/init") ||
>>
>> Can't you just make it use "init=foo,strict" instead?
>
> Can't we just change the default behavior and add a
> CONFIG_INIT_FALLBACK that defaults to "n" which you can switch on to
> get the old behavior? (And then immediately deprecate it?)
>
> If you're specifying an init, you probably want that init...

Hmm, that's a reasonable point.

Thoughts, anyone?  I'd be okay with doing that.

--Andy

>
> Rob



-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-09-26 19:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-26 19:13 [PATCH v3] init: Add strictinit to disable init= fallbacks Andy Lutomirski
2014-09-26 19:23 ` Chuck Ebbert
2014-09-26 19:30   ` Rob Landley
2014-09-26 19:32     ` Andy Lutomirski
2014-09-26 19:31   ` Andy Lutomirski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.