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

Groups > linux.kernel > #1560290 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  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

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

* 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

[toc] | [next] | [standalone]

#1561463

On Tuesday, January 17, 2017 8:18 AM, Ingo Molnar wrote: 
> Mind sending this as a proper patch, with akpm Cc:-ed?

Not at all, I'll put it together this week.

Thanks,
Lukas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web