linux-kernel - Re: [PATCH v4 03/36] x86/bugs: Restructure mmio 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] [day] [month] [year] [list]

Message-ID: <20250313192606.iijythngqtpx4tyy@desk>
Date: Thu, 13 Mar 2025 12:26:06 -0700
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: Borislav Petkov <bp@...en8.de>
Cc: David Kaplan <david.kaplan@....com>,
	Thomas Gleixner <tglx@...utronix.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,
	Brendan Jackman <jackmanb@...gle.com>,
	Derek Manwaring <derekmn@...zon.com>
Subject: Re: [PATCH v4 03/36] x86/bugs: Restructure mmio mitigation

On Thu, Mar 13, 2025 at 10:36:17AM +0100, Borislav Petkov wrote:
> I'd expect to see:
> 
> 	if (mmio_mitigation == MMIO_MITIGATION_AUTO) {
>                 mmio_mitigation = MMIO_MITIGATION_VERW;
> 		verw_mitigation_selected = true;
> 	}
> 
>         if (boot_cpu_has_bug(X86_BUG_MDS) || taa_vulnerable())
>                 verw_mitigation_selected = true;
> 
> because the above branch already selected MMIO_MITIGATION_VERW so we might as
> well set verw_mitigation_selected, right?

There is a subtle difference between setting verw_mitigation_selected and
MMIO_MITIGATION_VERW. The former is a system-wide switch that indicates
VERW is needed at both kernel-exit and VMenter. MMIO Stale Data is
different from other VERW based mitigations because it only requires VERW
at VMenter, when not affected by MDS/TAA. So, turning the system-wide knob
here would be wrong.

> > +static void __init mmio_update_mitigation(void)
> > +{
> > +	if (!boot_cpu_has_bug(X86_BUG_MMIO_STALE_DATA) || cpu_mitigations_off())
> > +		return;
> > +
> > +	if (verw_mitigation_selected)
> > +		mmio_mitigation = MMIO_MITIGATION_VERW;
[...]
> > +	if (boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN))
> 
> Btw, that UNKNOWN thing is just silly. Looking at git history:
> 
> 7df548840c49 ("x86/bugs: Add "unknown" reporting for MMIO Stale Data")
> 
> this was added just so that it doesn't say "Not affected" about those CPUs but
> "unknown."
> 
> But
> 
>   "Mitigation is not deployed when the status is unknown."
> 
> so if it is only about reporting, I think we can synthesize the logic of this:
> 
>         if (!arch_cap_mmio_immune(x86_arch_cap_msr)) {
>                 if (cpu_matches(cpu_vuln_blacklist, MMIO))
>                         setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);
>                 else if (!cpu_matches(cpu_vuln_whitelist, NO_MMIO))
>                         setup_force_cpu_bug(X86_BUG_MMIO_UNKNOWN);
>         }
> 
> into a separate function and get rid of that X86_BUG_MMIO_UNKNOWN thing.

Hmm, that would not be straightforward, specially for sysfs status. The
above logic requires parsing the cpu_vuln_whitelist which is not available
after init. Moreover, sysfs reads would become slower if it has to read an
MSR and parse tables.

Also, cpu_show_common() by default shows "Not affected" in the absence of
bug bit. So setting X86_BUG_MMIO_UNKNOWN is simpler overall.

cpu_show_common(bug)
{
	if (!boot_cpu_has_bug(bug))
		return sysfs_emit(buf, "Not affected\n");