linux-kernel - RE: [PATCH v5 16/16] x86/bugs: Restructure SRSO mitigation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <LV3PR12MB926560C299979A7606A7417494802@LV3PR12MB9265.namprd12.prod.outlook.com>
Date: Tue, 29 Apr 2025 17:18:27 +0000
From: "Kaplan, David" <David.Kaplan@....com>
To: Borislav Petkov <bp@...en8.de>
CC: Thomas Gleixner <tglx@...utronix.de>, Peter Zijlstra
	<peterz@...radead.org>, Josh Poimboeuf <jpoimboe@...nel.org>, Pawan Gupta
	<pawan.kumar.gupta@...ux.intel.com>, 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 v5 16/16] x86/bugs: Restructure SRSO mitigation

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Borislav Petkov <bp@...en8.de>
> Sent: Tuesday, April 29, 2025 11:51 AM
> To: Kaplan, David <David.Kaplan@....com>
> Cc: Thomas Gleixner <tglx@...utronix.de>; Peter Zijlstra <peterz@...radead.org>;
> Josh Poimboeuf <jpoimboe@...nel.org>; Pawan Gupta
> <pawan.kumar.gupta@...ux.intel.com>; 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 v5 16/16] x86/bugs: Restructure SRSO mitigation
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Fri, Apr 18, 2025 at 11:17:21AM -0500, David Kaplan wrote:
> > @@ -2738,130 +2730,80 @@ static void __init
> > srso_select_mitigation(void)  {
> >       bool has_microcode = boot_cpu_has(X86_FEATURE_IBPB_BRTYPE);
>
> I'll push that init after the return so that it doesn't happen unnecessarily.
>
> > -     if (!boot_cpu_has_bug(X86_BUG_SRSO) ||
> > -         cpu_mitigations_off() ||
> > -         srso_cmd == SRSO_CMD_OFF) {
> > -             if (boot_cpu_has(X86_FEATURE_SBPB))
> > -                     x86_pred_cmd = PRED_CMD_SBPB;
> > -             goto out;
> > -     }
> > +     if (!boot_cpu_has_bug(X86_BUG_SRSO) || cpu_mitigations_off())
> > +             srso_mitigation = SRSO_MITIGATION_NONE;
> > +
> > +     if (srso_mitigation == SRSO_MITIGATION_NONE)
> > +             return;
> > +
> > +     if (srso_mitigation == SRSO_MITIGATION_AUTO)
> > +             srso_mitigation = SRSO_MITIGATION_SAFE_RET;
> >
> >       if (has_microcode) {
> >               /*
> >                * Zen1/2 with SMT off aren't vulnerable after the right
> >                * IBPB microcode has been applied.
> > -              *
> > -              * Zen1/2 don't have SBPB, no need to try to enable it here.
>
> Why?
>

The comment doesn't make any sense in the new structure.  In the old code, SBPB gets enabled at the start of the function, before we check if you're on a Zen1/2 with SMT off.  The comment arguably made some sense in the old code because you're disabling SRSO mitigations but after you had done the SBPB check...but the comment is pointing out this is ok because these CPUs never support SBPB anyway.  Normally, if SRSO is off, you try to use SBPB.

In the new flow, the SBPB work is done in srso_apply_mitigation(), and for all parts.  So the whole concern about how the code determines SRSO mitigation isn't needed after already handling SBPB doesn't exist anymore.

I'd say this is another reason why the new structure is easier to understand, it has fewer subtleties like this.

--David Kaplan