Re: [PATCH] trace: fix return value of __setup handlers

[Masami Hiramatsu]

On Wed,  2 Mar 2022 19:17:44 -0800
Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:

> __setup() handlers should generally return 1 to indicate that the
> boot options have been handled.
> 
> Using invalid option values causes the entire kernel boot option
> string to be reported as Unknown and added to init's environment
> strings, polluting it.
> 
>   Unknown kernel command line parameters "BOOT_IMAGE=/boot/bzImage-517rc6
>     kprobe_event=p,syscall_any,$arg1 trace_options=quiet
>     trace_clock=jiffies", will be passed to user space.
> 
>  Run /sbin/init as init process
>    with arguments:
>      /sbin/init
>    with environment:
>      HOME=/
>      TERM=linux
>      BOOT_IMAGE=/boot/bzImage-517rc6
>      kprobe_event=p,syscall_any,$arg1
>      trace_options=quiet
>      trace_clock=jiffies
> 
> Return 1 from the __setup() handlers so that init's environment is not
> polluted with kernel boot options.

Oops, thanks for pointing it out.

Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

> 
> Fixes: 7bcfaf54f591 ("tracing: Add trace_options kernel command line parameter")
> Fixes: e1e232ca6b8f ("tracing: Add trace_clock=<clock> kernel parameter")
> Fixes: 970988e19eb0 ("tracing/kprobe: Add kprobe_event= boot parameter")
> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> Reported-by: Igor Zhbanov <i.zhbanov@xxxxxxxxxxxx>
> Link: lore.kernel.org/r/64644a2f-4a20-bab3-1e15-3b2cdd0defe3@xxxxxxxxxxxx
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
> "trace_clock=" reports invalid parameter usage later, when it tries
> to use the value:
>   Trace clock jiffies not defined, going back to default
> 
>  kernel/trace/trace.c        |    4 ++--
>  kernel/trace/trace_kprobe.c |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> --- linux-next-20220302.orig/kernel/trace/trace.c
> +++ linux-next-20220302/kernel/trace/trace.c
> @@ -235,7 +235,7 @@ static char trace_boot_options_buf[MAX_T
>  static int __init set_trace_boot_options(char *str)
>  {
>  	strlcpy(trace_boot_options_buf, str, MAX_TRACER_SIZE);
> -	return 0;
> +	return 1;
>  }
>  __setup("trace_options=", set_trace_boot_options);
>  
> @@ -246,7 +246,7 @@ static int __init set_trace_boot_clock(c
>  {
>  	strlcpy(trace_boot_clock_buf, str, MAX_TRACER_SIZE);
>  	trace_boot_clock = trace_boot_clock_buf;
> -	return 0;
> +	return 1;
>  }
>  __setup("trace_clock=", set_trace_boot_clock);
>  
> --- linux-next-20220302.orig/kernel/trace/trace_kprobe.c
> +++ linux-next-20220302/kernel/trace/trace_kprobe.c
> @@ -32,7 +32,7 @@ static int __init set_kprobe_boot_events
>  	strlcpy(kprobe_boot_events_buf, str, COMMAND_LINE_SIZE);
>  	disable_tracing_selftest("running kprobe events");
>  
> -	return 0;
> +	return 1;
>  }
>  __setup("kprobe_event=", set_kprobe_boot_events);
>  


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>