[PATCH] init: fix bug where environment vars can't be passed via boot args

[PATCH] init: fix bug where environment vars can't be passed via boot args

[Chris Metcalf]

Commit 026cee0086f had the side-effect of dropping the '=' from
the unknown boot arguments that are passed to init as environment
variables.  This is because parse_args() puts a NUL in the string
where the '=' was when it passes the "param" and "val" pointers
to the parsing subfunctions.  Previously, unknown_bootoption() was
the last parse_args() subfunction to run, and it carefully put back
the '=' character.  Now ignore_unknown_bootoption() is the last
one to run, and it wasn't doing the necessary repair, so the
envp params ended up with the embedded NUL and were no longer
seen as valid environment variables by init.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
 init/main.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/init/main.c b/init/main.c
index 9d454f0..44b2433 100644
--- a/init/main.c
+++ b/init/main.c
@@ -225,13 +225,9 @@ static int __init loglevel(char *str)
 
 early_param("loglevel", loglevel);
 
-/*
- * Unknown boot options get handed to init, unless they look like
- * unused parameters (modprobe will find them in /proc/cmdline).
- */
-static int __init unknown_bootoption(char *param, char *val)
+/* Change NUL term back to "=", to make "param" the whole string. */
+static int __init repair_env_string(char *param, char *val)
 {
-	/* Change NUL term back to "=", to make "param" the whole string. */
 	if (val) {
 		/* param=val or param="val"? */
 		if (val == param+strlen(param)+1)
@@ -243,6 +239,16 @@ static int __init unknown_bootoption(char *param, char *val)
 		} else
 			BUG();
 	}
+	return 0;
+}
+
+/*
+ * Unknown boot options get handed to init, unless they look like
+ * unused parameters (modprobe will find them in /proc/cmdline).
+ */
+static int __init unknown_bootoption(char *param, char *val)
+{
+	repair_env_string(param, val);
 
 	/* Handle obsolete-style parameters */
 	if (obsolete_checksetup(param))
@@ -732,11 +738,6 @@ static char *initcall_level_names[] __initdata = {
 	"late parameters",
 };
 
-static int __init ignore_unknown_bootoption(char *param, char *val)
-{
-	return 0;
-}
-
 static void __init do_initcall_level(int level)
 {
 	extern const struct kernel_param __start___param[], __stop___param[];
@@ -747,7 +748,7 @@ static void __init do_initcall_level(int level)
 		   static_command_line, __start___param,
 		   __stop___param - __start___param,
 		   level, level,
-		   ignore_unknown_bootoption);
+		   repair_env_string);
 
 	for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
 		do_one_initcall(*fn);
-- 
1.6.5.2

Re: [PATCH] init: fix bug where environment vars can't be passed via boot args

[Woody Suwalski]

Chris Metcalf wrote:
> Commit 026cee0086f had the side-effect of dropping the '=' from
> the unknown boot arguments that are passed to init as environment
> variables.  This is because parse_args() puts a NUL in the string
> where the '=' was when it passes the "param" and "val" pointers
> to the parsing subfunctions.  Previously, unknown_bootoption() was
> the last parse_args() subfunction to run, and it carefully put back
> the '=' character.  Now ignore_unknown_bootoption() is the last
> one to run, and it wasn't doing the necessary repair, so the
> envp params ended up with the embedded NUL and were no longer
> seen as valid environment variables by init.
>
> Signed-off-by: Chris Metcalf<cmetcalf@tilera.com>
> ---
>   init/main.c |   25 +++++++++++++------------
>   1 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 9d454f0..44b2433 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -225,13 +225,9 @@ static int __init loglevel(char *str)
>
>   early_param("loglevel", loglevel);
>
> -/*
> - * Unknown boot options get handed to init, unless they look like
> - * unused parameters (modprobe will find them in /proc/cmdline).
> - */
> -static int __init unknown_bootoption(char *param, char *val)
> +/* Change NUL term back to "=", to make "param" the whole string. */
> +static int __init repair_env_string(char *param, char *val)
>   {
> -	/* Change NUL term back to "=", to make "param" the whole string. */
>   	if (val) {
>   		/* param=val or param="val"? */
>   		if (val == param+strlen(param)+1)
> @@ -243,6 +239,16 @@ static int __init unknown_bootoption(char *param, char *val)
>   		} else
>   			BUG();
>   	}
> +	return 0;
> +}
> +
> +/*
> + * Unknown boot options get handed to init, unless they look like
> + * unused parameters (modprobe will find them in /proc/cmdline).
> + */
> +static int __init unknown_bootoption(char *param, char *val)
> +{
> +	repair_env_string(param, val);
>
>   	/* Handle obsolete-style parameters */
>   	if (obsolete_checksetup(param))
> @@ -732,11 +738,6 @@ static char *initcall_level_names[] __initdata = {
>   	"late parameters",
>   };
>
> -static int __init ignore_unknown_bootoption(char *param, char *val)
> -{
> -	return 0;
> -}
> -
>   static void __init do_initcall_level(int level)
>   {
>   	extern const struct kernel_param __start___param[], __stop___param[];
> @@ -747,7 +748,7 @@ static void __init do_initcall_level(int level)
>   		   static_command_line, __start___param,
>   		   __stop___param - __start___param,
>   		   level, level,
> -		   ignore_unknown_bootoption);
> +		   repair_env_string);
>
>   	for (fn = initcall_levels[level]; fn<  initcall_levels[level+1]; fn++)
>   		do_one_initcall(*fn);

Thanks for the patch, works for me...
Woody

Re: [PATCH] init: fix bug where environment vars can't be passed via boot args

[Pawel Moll]

On Fri, 2012-04-06 at 17:53 +0100, Chris Metcalf wrote:
> Commit 026cee0086f had the side-effect of dropping the '=' from
> the unknown boot arguments that are passed to init as environment
> variables.  This is because parse_args() puts a NUL in the string
> where the '=' was when it passes the "param" and "val" pointers
> to the parsing subfunctions.  Previously, unknown_bootoption() was
> the last parse_args() subfunction to run, and it carefully put back
> the '=' character.  Now ignore_unknown_bootoption() is the last
> one to run, and it wasn't doing the necessary repair, so the
> envp params ended up with the embedded NUL and were no longer
> seen as valid environment variables by init.
> 
> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>

Looks sensible to me.

Acked-by: Pawel Moll <pawel.moll@arm.com>

Thanks and apologies about delayed answer (and for causing the problem
in the first place :-)

Paweł

Re: [PATCH] init: fix bug where environment vars can't be passed via boot args

[Andrew Morton]

On Fri, 6 Apr 2012 12:53:50 -0400
Chris Metcalf <cmetcalf@tilera.com> wrote:

> Commit 026cee0086f had the side-effect of dropping the '=' from
> the unknown boot arguments that are passed to init as environment
> variables.  This is because parse_args() puts a NUL in the string
> where the '=' was when it passes the "param" and "val" pointers
> to the parsing subfunctions.  Previously, unknown_bootoption() was
> the last parse_args() subfunction to run, and it carefully put back
> the '=' character.  Now ignore_unknown_bootoption() is the last
> one to run, and it wasn't doing the necessary repair, so the
> envp params ended up with the embedded NUL and were no longer
> seen as valid environment variables by init.

This patch has been stuck in your tree for a week or two.  The copy
there is missing Woody's Tested-by and (I suspect) his Reported-by and
Pawel's Acked-by.

And because that patch is already in linux-next, I can't (or rather
don't want to) merge it myself.

It needs to be merged into 3.4.  Please fix up the changelog data and
send it in?

Re: [PATCH] init: fix bug where environment vars can't be passed via boot args

[Chris Metcalf]

On 4/20/2012 12:17 AM, Andrew Morton wrote:
> On Fri, 6 Apr 2012 12:53:50 -0400
> Chris Metcalf <cmetcalf@tilera.com> wrote:
>
>> Commit 026cee0086f had the side-effect of dropping the '=' from
>> the unknown boot arguments that are passed to init as environment
>> variables.  This is because parse_args() puts a NUL in the string
>> where the '=' was when it passes the "param" and "val" pointers
>> to the parsing subfunctions.  Previously, unknown_bootoption() was
>> the last parse_args() subfunction to run, and it carefully put back
>> the '=' character.  Now ignore_unknown_bootoption() is the last
>> one to run, and it wasn't doing the necessary repair, so the
>> envp params ended up with the embedded NUL and were no longer
>> seen as valid environment variables by init.
> This patch has been stuck in your tree for a week or two.  The copy
> there is missing Woody's Tested-by and (I suspect) his Reported-by and
> Pawel's Acked-by.

I pushed it by accident - sorry.  I'll double-check my pending changes more
carefully in the future.  I meant for that tree only to hold tile-specific
stuff, but I had the boot-argument change in my tree so I could do more
effective testing.  I'm on vacation till Tuesday in Croatia, unfortunately,
so I don't really want to try to do more than email.

> And because that patch is already in linux-next, I can't (or rather
> don't want to) merge it myself.
>
> It needs to be merged into 3.4.  Please fix up the changelog data and
> send it in?

I will either fix it up and move it to my stable tree for 3.4 on Wednesday
when I'm back in the office, and ask Linus to pull it, or if you want to
push it, presumably it shouldn't conflict much for linux-next.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH] init: fix bug where environment vars can't be passed via boot args

[Chris Metcalf]

On 4/19/2012 6:17 PM, Andrew Morton wrote:
> On Fri, 6 Apr 2012 12:53:50 -0400
> Chris Metcalf <cmetcalf@tilera.com> wrote:
>
>> Commit 026cee0086f had the side-effect of dropping the '=' from
>> the unknown boot arguments that are passed to init as environment
>> variables.  This is because parse_args() puts a NUL in the string
>> where the '=' was when it passes the "param" and "val" pointers
>> to the parsing subfunctions.  Previously, unknown_bootoption() was
>> the last parse_args() subfunction to run, and it carefully put back
>> the '=' character.  Now ignore_unknown_bootoption() is the last
>> one to run, and it wasn't doing the necessary repair, so the
>> envp params ended up with the embedded NUL and were no longer
>> seen as valid environment variables by init.
> This patch has been stuck in your tree for a week or two.  The copy
> there is missing Woody's Tested-by and (I suspect) his Reported-by and
> Pawel's Acked-by.
>
> And because that patch is already in linux-next, I can't (or rather
> don't want to) merge it myself.
>
> It needs to be merged into 3.4.  Please fix up the changelog data and
> send it in?

I pulled it out of my linux-next tree and into my stable tree (for -rc
changes).  I'll send Linus a pull request now.  Thanks for noticing.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com