[PATCH] init: Make unknown command line param message clearer

[PATCH] init: Make unknown command line param message clearer

[Andrew Halaney]

The prior message is confusing users, which is the exact opposite of the
goal. If the message is being seen, one of the following situations is
happening:

 1. the param is misspelled
 2. the param is not valid due to the kernel configuration
 3. the param is intended for init but isn't after the '--'
    delineator on the command line

To make that more clear to the user, explicitly mention "kernel command
line" and also note that the params are still passed to user space to
avoid causing any alarm over params intended for init.

Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

Here's v2 of this. I had to change the code a bit from what Steven
recommended to compile/look proper, but the intended format he had
suggested is still the same.

v1 -> v2:
 * Print a much more concise message

v1: https://lore.kernel.org/all/20211012213523.39801-1-ahalaney@redhat.com/

Thanks,
Andrew

 init/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/init/main.c b/init/main.c
index ee4d3e1b3eb9..a286995449e2 100644
--- a/init/main.c
+++ b/init/main.c
@@ -925,7 +925,9 @@ static void __init print_unknown_bootoptions(void)
 	for (p = &envp_init[2]; *p; p++)
 		end += sprintf(end, " %s", *p);
 
-	pr_notice("Unknown command line parameters:%s\n", unknown_options);
+	/* Start at unknown_options[1] to skip the initial space */
+	pr_notice("Unknown kernel command line parameters \"%s\", will be passed to user space.\n",
+		&unknown_options[1]);
 	memblock_free(unknown_options, len);
 }
 
-- 
2.31.1

Re: [PATCH] init: Make unknown command line param message clearer

[Randy Dunlap]

On 10/13/21 3:35 PM, Andrew Halaney wrote:
> The prior message is confusing users, which is the exact opposite of the
> goal. If the message is being seen, one of the following situations is
> happening:
> 
>   1. the param is misspelled
>   2. the param is not valid due to the kernel configuration
>   3. the param is intended for init but isn't after the '--'
>      delineator on the command line
> 
> To make that more clear to the user, explicitly mention "kernel command
> line" and also note that the params are still passed to user space to
> avoid causing any alarm over params intended for init.
> 
> Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
> Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> ---
> 
> Here's v2 of this. I had to change the code a bit from what Steven
> recommended to compile/look proper, but the intended format he had
> suggested is still the same.
> 
> v1 -> v2:
>   * Print a much more concise message
> 
> v1: https://lore.kernel.org/all/20211012213523.39801-1-ahalaney@redhat.com/
> 
> Thanks,
> Andrew
> 
>   init/main.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/init/main.c b/init/main.c
> index ee4d3e1b3eb9..a286995449e2 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -925,7 +925,9 @@ static void __init print_unknown_bootoptions(void)
>   	for (p = &envp_init[2]; *p; p++)
>   		end += sprintf(end, " %s", *p);
>   
> -	pr_notice("Unknown command line parameters:%s\n", unknown_options);
> +	/* Start at unknown_options[1] to skip the initial space */
> +	pr_notice("Unknown kernel command line parameters \"%s\", will be passed to user space.\n",
> +		&unknown_options[1]);
>   	memblock_free(unknown_options, len);
>   }
>   
> 

LGTM. Thanks.

Acked-by: Randy Dunlap <rdunlap@infradead.org>

-- 
~Randy

Re: [PATCH] init: Make unknown command line param message clearer

[Andrew Morton]

On Wed, 13 Oct 2021 17:35:02 -0500 Andrew Halaney <ahalaney@redhat.com> wrote:

> The prior message is confusing users, which is the exact opposite of the
> goal. If the message is being seen, one of the following situations is
> happening:
> 
>  1. the param is misspelled
>  2. the param is not valid due to the kernel configuration
>  3. the param is intended for init but isn't after the '--'
>     delineator on the command line
> 
> To make that more clear to the user, explicitly mention "kernel command
> line" and also note that the params are still passed to user space to
> avoid causing any alarm over params intended for init.
> 
> Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
> Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>

Thanks.

BTW, I'm still sitting on your "init/main.c: silence some
-Wunused-parameter warnings", awaiting a response to Rasmus's
suggestion:
https://lkml.kernel.org/r/f06b8308-645b-031b-f9c6-b92400a269aa@rasmusvillemoes.dk

Re: [PATCH] init: Make unknown command line param message clearer

[Andrew Halaney]

On Wed, Oct 13, 2021 at 04:12:59PM -0700, Andrew Morton wrote:
> On Wed, 13 Oct 2021 17:35:02 -0500 Andrew Halaney <ahalaney@redhat.com> wrote:
> 
> > The prior message is confusing users, which is the exact opposite of the
> > goal. If the message is being seen, one of the following situations is
> > happening:
> > 
> >  1. the param is misspelled
> >  2. the param is not valid due to the kernel configuration
> >  3. the param is intended for init but isn't after the '--'
> >     delineator on the command line
> > 
> > To make that more clear to the user, explicitly mention "kernel command
> > line" and also note that the params are still passed to user space to
> > avoid causing any alarm over params intended for init.
> > 
> > Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
> > Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> 
> Thanks.
> 
> BTW, I'm still sitting on your "init/main.c: silence some
> -Wunused-parameter warnings", awaiting a response to Rasmus's
> suggestion:
> https://lkml.kernel.org/r/f06b8308-645b-031b-f9c6-b92400a269aa@rasmusvillemoes.dk
> 

Whoops, that got caught in spam (along with rostedt's replies
yesterday... darn gmail). Let me add looking at that to the todo list,
thanks Andrew!

Re: [PATCH] init: Make unknown command line param message clearer

[Borislav Petkov]

On Wed, Oct 13, 2021 at 09:03:33AM -0400, Steven Rostedt wrote:
> Just enough info that makes sense, and have comments or documentation
> elsewhere that can explain more details.

Yes, as we do, for example in:

f77f420d3475 ("x86/msr: Add a pointer to an URL which contains further details")

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg

Re: [PATCH] init: Make unknown command line param message clearer

[Steven Rostedt]

On Wed, 13 Oct 2021 07:56:38 -0500
Andrew Halaney <ahalaney@redhat.com> wrote:

> I'll spin a v2 with that considering everyone likes that form more.

Right, because we don't need a novel in the kernel to be printed ;-)

Just enough info that makes sense, and have comments or documentation
elsewhere that can explain more details.

-- Steve

Re: [PATCH] init: Make unknown command line param message clearer

[Andrew Halaney]

On Tue, Oct 12, 2021 at 05:18:32PM -0700, Randy Dunlap wrote:
> On 10/12/21 5:01 PM, Steven Rostedt wrote:
> > On Tue, 12 Oct 2021 16:35:23 -0500
> > Andrew Halaney <ahalaney@redhat.com> wrote:
> > 
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
> > >   	for (p = &envp_init[2]; *p; p++)
> > >   		end += sprintf(end, " %s", *p);
> > > +	pr_notice("The kernel command line has unknown parameters. They are either\n");
> > > +	pr_notice("misspelled, not valid for the current kernel configuration,\n");
> > > +	pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
> > > +	pr_notice("be passed to init along with those after '--' on the command line.\n");
> > >   	pr_notice("Unknown command line parameters:%s\n", unknown_options);
> > >   	memblock_free(unknown_options, len);
> > 
> > What about just changing it to simply say:
> > 
> > 	pr_notice("Unknown kernel command line parameters "%s", will be	passed to user space.\n",
> > 		  unknown_options);
> > 
> 
> Yes, that's much more palatable.
> 
> thanks.
> -- 
> ~Randy
> 

Heh, that's basically what the users suggested too but I wasn't the
biggest fan since I didn't think it highlighted the points I tried to
make. I guess I'm just too verbose :P

I'll spin a v2 with that considering everyone likes that form more.

Thanks,
Andrew

Re: [PATCH] init: Make unknown command line param message clearer

[Steven Rostedt]

On Tue, 12 Oct 2021 17:18:32 -0700
Randy Dunlap <rdunlap@infradead.org> wrote:

> On 10/12/21 5:01 PM, Steven Rostedt wrote:
> > On Tue, 12 Oct 2021 16:35:23 -0500
> > Andrew Halaney <ahalaney@redhat.com> wrote:
> >   
> >> --- a/init/main.c
> >> +++ b/init/main.c
> >> @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
> >>   	for (p = &envp_init[2]; *p; p++)
> >>   		end += sprintf(end, " %s", *p);
> >>   
> >> +	pr_notice("The kernel command line has unknown parameters. They are either\n");
> >> +	pr_notice("misspelled, not valid for the current kernel configuration,\n");
> >> +	pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
> >> +	pr_notice("be passed to init along with those after '--' on the command line.\n");
> >>   	pr_notice("Unknown command line parameters:%s\n", unknown_options);
> >>   	memblock_free(unknown_options, len);  
> > 
> > What about just changing it to simply say:
> > 
> > 	pr_notice("Unknown kernel command line parameters "%s", will be	passed to user space.\n",
> > 		  unknown_options);
> >   
> 
> Yes, that's much more palatable.

Thanks.

Andrew (Halaney, not Morton),

Feel free to send a v2 with the above text, and just add:

Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

Re: [PATCH] init: Make unknown command line param message clearer

[Randy Dunlap]

On 10/12/21 5:01 PM, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 16:35:23 -0500
> Andrew Halaney <ahalaney@redhat.com> wrote:
> 
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
>>   	for (p = &envp_init[2]; *p; p++)
>>   		end += sprintf(end, " %s", *p);
>>   
>> +	pr_notice("The kernel command line has unknown parameters. They are either\n");
>> +	pr_notice("misspelled, not valid for the current kernel configuration,\n");
>> +	pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
>> +	pr_notice("be passed to init along with those after '--' on the command line.\n");
>>   	pr_notice("Unknown command line parameters:%s\n", unknown_options);
>>   	memblock_free(unknown_options, len);
> 
> What about just changing it to simply say:
> 
> 	pr_notice("Unknown kernel command line parameters "%s", will be	passed to user space.\n",
> 		  unknown_options);
> 

Yes, that's much more palatable.

thanks.
-- 
~Randy

Re: [PATCH] init: Make unknown command line param message clearer

[Steven Rostedt]

On Tue, 12 Oct 2021 16:35:23 -0500
Andrew Halaney <ahalaney@redhat.com> wrote:

> --- a/init/main.c
> +++ b/init/main.c
> @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
>  	for (p = &envp_init[2]; *p; p++)
>  		end += sprintf(end, " %s", *p);
>  
> +	pr_notice("The kernel command line has unknown parameters. They are either\n");
> +	pr_notice("misspelled, not valid for the current kernel configuration,\n");
> +	pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
> +	pr_notice("be passed to init along with those after '--' on the command line.\n");
>  	pr_notice("Unknown command line parameters:%s\n", unknown_options);
>  	memblock_free(unknown_options, len);

What about just changing it to simply say:

	pr_notice("Unknown kernel command line parameters "%s", will be	passed to user space.\n",
		  unknown_options);

-- Steve

[PATCH] init: Make unknown command line param message clearer

[Andrew Halaney]

The prior message is confusing users, which is the exact opposite of the
goal. Try and make it clear (without needing to look at the kernel
source) that the message is indicating one of the following
situations:

 1. the param is misspelled
 2. the param is not valid due to the kernel configuration
 3. the param is intended for init but isn't after the '--'
    delineator on the command line

On that same topic, also make it clear that these params are passed to
init still despite not being after the delineator.

Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

I'm not a huge fan of the wall of text this adds, but at the same time
I've had a few people come to me with confusion about the message and
concern that userspace isn't getting the params (not the case here, it's
just a cosmetic message). I'm open to better ideas on how to express
what I describe in the commit message, or if people think the message is
more confusing than useful a full revert would be ok with me too
(although I do think it is useful personally).

Thanks,
Andrew

 init/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/init/main.c b/init/main.c
index ee4d3e1b3eb9..8dc88c2386ee 100644
--- a/init/main.c
+++ b/init/main.c
@@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
 	for (p = &envp_init[2]; *p; p++)
 		end += sprintf(end, " %s", *p);
 
+	pr_notice("The kernel command line has unknown parameters. They are either\n");
+	pr_notice("misspelled, not valid for the current kernel configuration,\n");
+	pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
+	pr_notice("be passed to init along with those after '--' on the command line.\n");
 	pr_notice("Unknown command line parameters:%s\n", unknown_options);
 	memblock_free(unknown_options, len);
 }
-- 
2.31.1