linux-kernel - Re: [RFC PATCH 27/34] x86/bugs: Add attack vector controls for spectre

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: <8a406835-b985-415a-a944-25d0ebea4fd0@intel.com>
Date: Thu, 12 Sep 2024 12:37:22 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: David Kaplan <david.kaplan@....com>, Thomas Gleixner
 <tglx@...utronix.de>, Borislav Petkov <bp@...en8.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>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 27/34] x86/bugs: Add attack vector controls for
 spectre_v1

On 9/12/24 12:08, David Kaplan wrote:
> @@ -1114,6 +1114,9 @@ static void __init spectre_v1_select_mitigation(void)
>  {
>  	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) || cpu_mitigations_off())
>  		spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;
> +
> +	if (!should_mitigate_vuln(SPECTRE_V1))
> +		spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;
>  }

Just a high-level comment on this: usually in a well-structured series
that has sufficient refactoring, if you start to look at the end of the
series, things start to fall into place.  The series (at some point)
stops adding complexity things get simpler.

I don't really see that inflection point here.

For instance, I would have expected cpu_mitigations_off() to be
consulted in should_mitigate_vuln() so that some of the individual sites
can go away.

There's also added complexity from having 'enum vulnerabilities' which
basically duplicate the X86_BUG_* space.  If the infrastructure was, for
instance, built around X86_BUG bits, it might have enabled this patch to
be something like:

-  	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) ||
-	    cpu_mitigations_off())
+	if (!should_mitigate_vuln(X86_BUG_SPECTRE_V1))
		spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;

I'm also not sure this series takes the right approach in representing
logic in data structures versus code.

For instance, this:

> +	case MDS:
> +	case TAA:
> +	case MMIO:
> +	case RFDS:
> +	case SRBDS:
> +	case GDS:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);

We've _tended_ to represent these in data structure like cpu_vuln_whitelist.

struct whatever var[] =
   MACRO(MDS,  USER_KERNEL | GUEST_HOST | USER_USER | GUEST_GUEST)
   MACRO(MMIO, USER_KERNEL | GUEST_HOST | USER_USER | GUEST_GUEST)
   ...
};

But I do like the concept of users being focused on the attack vectors
in general.  That part is really nice.

As we talk about this at Plumbers, we probably need to be focused on
whether users want this new attack-vector-based selection mechanism or
the old style.  Because adding the attack-vector style is going to add
complexity any way we do it.