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


Groups > linux.kernel > #1560290

Re: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line option

From Ingo Molnar <mingo@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line option
Date 2017-01-17 08:20 +0100
Message-ID <t0wdH-41a-7@gated-at.bofh.it> (permalink)
References <sTmVP-tN-1@gated-at.bofh.it> <sTG8a-5mg-11@gated-at.bofh.it> <sWb7Q-8fR-27@gated-at.bofh.it> <t0kFz-4uU-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


* Odzioba, Lukasz <lukasz.odzioba@intel.com> wrote:

> >	pr_warn("x86/cpu: Ignoring invalid "clearcpuid=%s' option!\n", arg)
> >
> > Which would save quite a bit of head scratching and frustration when someone has a 
> > bad enough day to add silly typos to the kernel cmdline.
> 
> Is there any particular reason why we have such warnings only for early params?
> early_param handlers return non-zero values on success:
> 	linux/init.h: " * Emits warning if fn returns non-zero."
> __setup handlers in most cases seem to return 1 on success, is the expected
> behaviour documented somewhere?
> 
> After looking at some of the ~500 usages of __setup macro it seems that handler's ret
> code doesn't matter so much, because it is treated differently in various parts
> of the kernel. If we make it consistent possibly it could be solved similarly to 
> early params by something like this: 
> 
> diff --git a/init/main.c b/init/main.c
> index b0c9d6f..261178e 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -182,8 +182,12 @@ static bool __init obsolete_checksetup(char *line)
>                                 pr_warn("Parameter %s is obsolete, ignored\n",
>                                         p->str);
>                                 return true;
> -                       } else if (p->setup_func(line + n))
> -                               return true;
> +                       } else {
> +                               if (p->setup_func(line + n))
> +                                       return true;
> +                               else
> +                                       pr_warn("Malformed option '%s'\n", line);
> +                       }

That looks sensible to me! I'd tweak the message slightly:

	pr_warn("error: Ignoring invalid boot parameter '%s'\n", line);

to make it more clear that it's a boot option that has a problem (there are many 
other types of options), and to make sure the user knows that we ignored that 
option.

Mind sending this as a proper patch, with akpm Cc:-ed?

Thanks,

	Ingo

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line  option Ingo Molnar <mingo@kernel.org> - 2017-01-05 09:00 +0100
  RE: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line  option "Odzioba, Lukasz" <lukasz.odzioba@intel.com> - 2017-01-16 20:00 +0100
    Re: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line  option Ingo Molnar <mingo@kernel.org> - 2017-01-17 08:20 +0100
      RE: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line  option "Odzioba, Lukasz" <lukasz.odzioba@intel.com> - 2017-01-18 11:40 +0100

csiph-web