| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<LV3PR12MB9265D5129F8090B003C807BB9435A@LV3PR12MB9265.namprd12.prod.outlook.com>
Date: Thu, 14 Aug 2025 13:51:57 +0000
From: "Kaplan, David" <David.Kaplan@....com>
To: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
Peter Zijlstra <peterz@...radead.org>, Josh Poimboeuf <jpoimboe@...nel.org>,
Ingo Molnar <mingo@...hat.com>, Dave Hansen <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "H . Peter Anvin" <hpa@...or.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/3] x86/bugs: Use early_param for spectre_v2_user
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Sent: Wednesday, August 13, 2025 10:20 PM
> To: Kaplan, David <David.Kaplan@....com>
> Cc: Thomas Gleixner <tglx@...utronix.de>; Borislav Petkov <bp@...en8.de>; Peter
> Zijlstra <peterz@...radead.org>; Josh Poimboeuf <jpoimboe@...nel.org>; Ingo
> Molnar <mingo@...hat.com>; Dave Hansen <dave.hansen@...ux.intel.com>;
> x86@...nel.org; H . Peter Anvin <hpa@...or.com>; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH 1/3] x86/bugs: Use early_param for spectre_v2_user
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Mon, Aug 11, 2025 at 09:26:57AM -0500, David Kaplan wrote:
> > Most of the mitigations in bugs.c use early_param to parse their command
> > line options. Modify spectre_v2_user to use early_param for consistency.
> >
> > Signed-off-by: David Kaplan <david.kaplan@....com>
> > ---
> > arch/x86/kernel/cpu/bugs.c | 62 ++++++++++++++++++--------------------
> > 1 file changed, 30 insertions(+), 32 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index b74bf937cd9f..6bfe199b9f3e 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -1829,7 +1829,7 @@ enum spectre_v2_mitigation_cmd {
> >
> > static enum spectre_v2_mitigation_cmd spectre_v2_cmd __ro_after_init =
> SPECTRE_V2_CMD_AUTO;
> >
> > -enum spectre_v2_user_cmd {
> > +enum spectre_v2_user_mitigation_cmd {
> > SPECTRE_V2_USER_CMD_NONE,
> > SPECTRE_V2_USER_CMD_AUTO,
> > SPECTRE_V2_USER_CMD_FORCE,
> > @@ -1839,6 +1839,9 @@ enum spectre_v2_user_cmd {
> > SPECTRE_V2_USER_CMD_SECCOMP_IBPB,
> > };
> >
> > +static enum spectre_v2_user_mitigation_cmd spectre_v2_user_cmd
> __ro_after_init =
> > + SPECTRE_V2_USER_CMD_AUTO;
> > +
> > static const char * const spectre_v2_user_strings[] = {
> > [SPECTRE_V2_USER_NONE] = "User space: Vulnerable",
> > [SPECTRE_V2_USER_STRICT] = "User space: Mitigation: STIBP
> protection",
> > @@ -1847,50 +1850,45 @@ static const char * const spectre_v2_user_strings[]
> = {
> > [SPECTRE_V2_USER_SECCOMP] = "User space: Mitigation:
> STIBP via seccomp and prctl",
> > };
> >
> > -static const struct {
> > - const char *option;
> > - enum spectre_v2_user_cmd cmd;
> > - bool secure;
> > -} v2_user_options[] __initconst = {
> > - { "auto", SPECTRE_V2_USER_CMD_AUTO, false },
> > - { "off", SPECTRE_V2_USER_CMD_NONE, false },
> > - { "on", SPECTRE_V2_USER_CMD_FORCE, true },
> > - { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false },
> > - { "prctl,ibpb", SPECTRE_V2_USER_CMD_PRCTL_IBPB, false },
> > - { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false },
> > - { "seccomp,ibpb", SPECTRE_V2_USER_CMD_SECCOMP_IBPB,
> false },
> > -};
> > -
> > static void __init spec_v2_user_print_cond(const char *reason, bool secure)
> > {
> > if (boot_cpu_has_bug(X86_BUG_SPECTRE_V2) != secure)
> > pr_info("spectre_v2_user=%s forced on command line.\n", reason);
> > }
> >
> > -static enum spectre_v2_user_cmd __init spectre_v2_parse_user_cmdline(void)
> > +static int __init spectre_v2_parse_user_cmdline(char *str)
> > {
> > - char arg[20];
> > - int ret, i;
> > + if (!str)
> > + return -EINVAL;
> >
> > if (!IS_ENABLED(CONFIG_MITIGATION_SPECTRE_V2))
> > return SPECTRE_V2_USER_CMD_NONE;
> >
> > - ret = cmdline_find_option(boot_command_line, "spectre_v2_user",
> > - arg, sizeof(arg));
> > - if (ret < 0)
> > - return SPECTRE_V2_USER_CMD_AUTO;
> > + if (!strcmp(str, "auto"))
> > + spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_AUTO;
> > + else if (!strcmp(str, "off"))
> > + spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_NONE;
> > + else if (!strcmp(str, "on"))
> > + spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_FORCE;
> > + else if (!strcmp(str, "prctl"))
> > + spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_PRCTL;
> > + else if (!strcmp(str, "prctl,ibpb"))
> > + spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_PRCTL_IBPB;
> > + else if (!strcmp(str, "seccomp"))
> > + spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_SECCOMP;
> > + else if (!strcmp(str, "seccomp,ibpb"))
> > + spectre_v2_user_cmd =
> SPECTRE_V2_USER_CMD_SECCOMP_IBPB;
> > + else
> > + pr_err("Ignoring unknown spectre_v2_user option (%s).", str);
>
> Should return from here? Otherwise, spec_v2_user_print_cond() will print
> the unknown option as forced:
Yes, good point.
>
> pr_info("spectre_v2_user=%s forced on command line.\n", reason);
>
> >
> > - for (i = 0; i < ARRAY_SIZE(v2_user_options); i++) {
> > - if (match_option(arg, ret, v2_user_options[i].option)) {
> > - spec_v2_user_print_cond(v2_user_options[i].option,
> > - v2_user_options[i].secure);
> > - return v2_user_options[i].cmd;
> > - }
> > - }
> > + if (spectre_v2_user_cmd == SPECTRE_V2_USER_CMD_FORCE)
> > + spec_v2_user_print_cond(str, true);
> > + else
> > + spec_v2_user_print_cond(str, false);
>
> I don't see the need for spec_v2_user_print_cond(), should it be zapped?
>
> And then just do:
>
> if (spectre_v2_user_cmd != SPECTRE_V2_USER_CMD_FORCE)
> pr_info("spectre_v2_user=%s forced on command line.\n", str);
>
> I also feel that the original print is a bit confusing (code-wise), because
> it prints "forced" when the user opts for anything other than
> "on"(CMD_FORCE). I think the intent was to inform the user that a partially
> secure option is chosen.
>
I found that function confusing as well. None of the other bugs print a message just because you selected some command line option. Nor do they even print a message if you force a bug on (like indirect_target_selection=force) explicitly.
The fact is that we already print the selected mitigation at the end of update_mitigation function (like we do for all the bugs). I'm not sure why informing the user about the cmdline option they picked is of any value.
So I also would prefer to just eliminate the function entirely unless there are objections.
--David Kaplan
Powered by blists - more mailing lists