From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751282AbdAQHRq (ORCPT ); Tue, 17 Jan 2017 02:17:46 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:36496 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751196AbdAQHRo (ORCPT ); Tue, 17 Jan 2017 02:17:44 -0500 Date: Tue, 17 Jan 2017 08:17:39 +0100 From: Ingo Molnar To: "Odzioba, Lukasz" Cc: Borislav Petkov , "linux-kernel@vger.kernel.org" , "tglx@linutronix.de" , "mingo@redhat.com" , "hpa@zytor.com" , "x86@kernel.org" , "luto@kernel.org" , "slaoub@gmail.com" , "bp@suse.de" , "dave.hansen@linux.intel.com" , "Kleen, Andi" , Andrew Morton Subject: Re: [PATCH 1/1] x86: sanitize argument of clearcpuid command-line option Message-ID: <20170117071738.GA3041@gmail.com> References: <1482933340-11857-1-git-send-email-lukasz.odzioba@intel.com> <20161229102105.GD11221@nazgul.tnic> <20170105075559.GA2098@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Odzioba, Lukasz 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