[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<LV3PR12MB9265FD7451212DFA64F8B4009440A@LV3PR12MB9265.namprd12.prod.outlook.com>
Date: Wed, 2 Jul 2025 18:24:31 +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 14/20] x86/bugs: Add attack vector controls for BHI
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Borislav Petkov <bp@...en8.de>
> Sent: Tuesday, July 1, 2025 6:46 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 14/20] x86/bugs: Add attack vector controls for BHI
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Tue, Jul 01, 2025 at 03:27:00AM +0000, Kaplan, David wrote:
> > Eh, I'm not a big fan of that. It's basically overloading an existing
> > mitigation setting with a new meaning depending something else. That's
> > unnecessarily complex imo, and in this case is actually incorrect. The
> > attack vectors are supposed to be lower priority than bug-specific command
> > line options (as attack vectors are generic). So if you pass in
> > "spectre_bhi=vmexit" for instance, that should only mitigate bhi for vmexit,
> > even if you have general user->kernel protections enabled. The code below
> > appears to not observe that correctly.
> >
> > That issue aside, the enums I believe should ideally map to specific
> > mitigation decisions. There are two potential mitigations for BHI, so it
> > makes sense to have mitigation choices for all 4 potential outcomes.
>
> But then having a _FULL enum for both while *also* having USER_KERNEL and
> GUEST_HOST which both mean _FULL is kinda confusing, no?
I guess I don't see that. Those others don't mean full, they only protect the relevant attack vector. FULL protects all attack vectors.
>
> And when you look at the final code, that's kinda not really easy to grok for
> something which is actually simple.
>
> So I'm not sure the renaming and adding of another mitigation option is really
> needed. If you want to set mitigations based on the vectors, you can simply
> do:
>
> if (cpu_attack_vector_mitigated(CPU_MITIGATE_GUEST_HOST)) {
> if (cpu_attack_vector_mitigated(CPU_MITIGATE_USER_KERNEL)) {
> bhi_mitigation = BHI_MITIGATION_ON;
> else
> bhi_mitigation = BHI_MITIGATION_VMEXIT_ONLY;
> }
> }
>
> no?
>
> Using the existing switches.
>
> Because you have VMEXIT_ONLY already, so you check that first and only if
> USER_KERNEL is also mitigated, you select _ON.
>
> And then you don't need to touch bhi_apply_mitigation() at all.
>
> Or am I missing a corner case?
>
Yeah, I think my code was doing two things...it was adding the attack vector controls for the existing mitigations, and then also adding a new user->kernel only mitigation.
So probably the right answer here is to split this up. I'll change the patch to just use the existing mitigations and structure it like your snippet above. If someone wants to add a new user->kernel only option for BHI, that can be done in a separate patch later. There's actually probably several other mitigations that could similarly be split up based on attack vector (e.g. have separate controls for VERW in various places), if we wanted to have more mitigation options based on attack vectors (instead of just a simple on/off).
--David Kaplan
Powered by blists - more mailing lists