linux-kernel - Re: [PATCH v5 14/20] x86/bugs: Add attack vector controls for BHI

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: <20250701114537.GAaGPKYTbiLl4ABJ0l@fat_crate.local>
Date: Tue, 1 Jul 2025 13:45:37 +0200
From: Borislav Petkov <bp@...en8.de>
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" <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

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?

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?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette