The function should_mitigate_vuln() defines which vulnerabilities should
be mitigated based on the selected attack vector controls.  The
selections here are based on the individual characteristics of each
vulnerability.

Signed-off-by: David Kaplan <david.kaplan@amd.com>
---
 arch/x86/kernel/cpu/bugs.c | 69 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 88eba8e4c7fb..175dbbf9b06e 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -347,6 +347,75 @@ static void x86_amd_ssb_disable(void)
 		wrmsrl(MSR_AMD64_LS_CFG, msrval);
 }
 
+/*
+ * Returns true if vulnerability should be mitigated based on the
+ * selected attack vector controls
+ *
+ * See Documentation/admin-guide/hw-vuln/attack_vector_controls.rst
+ */
+static bool __init should_mitigate_vuln(unsigned int bug)
+{
+	switch (bug) {
+	/*
+	 * The only spectre_v1 mitigations in the kernel are related to
+	 * SWAPGS protection on kernel entry.  Therefore, protection is
+	 * only required for the user->kernel attack vector.
+	 */
+	case X86_BUG_SPECTRE_V1:
+		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL);
+
+	/*
+	 * Both spectre_v2 and srso may allow user->kernel or
+	 * guest->host attacks through branch predictor manipulation.
+	 */
+	case X86_BUG_SPECTRE_V2:
+	case X86_BUG_SRSO:
+		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL) ||
+			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
+
+	/*
+	 * spectre_v2_user refers to user->user or guest->guest branch
+	 * predictor attacks only.  Other indirect branch predictor attacks
+	 * are covered by the spectre_v2 vulnerability.
+	 */
+	case X86_BUG_SPECTRE_V2_USER:
+		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
+			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);
+
+	/* L1TF is only possible as a guest->host attack */
+	case X86_BUG_L1TF:
+		return cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
+
+	/*
+	 * All the vulnerabilities below allow potentially leaking data
+	 * across address spaces.  Therefore, mitigation is required for
+	 * any of these 4 attack vectors.
+	 */
+	case X86_BUG_MDS:
+	case X86_BUG_TAA:
+	case X86_BUG_MMIO_STALE_DATA:
+	case X86_BUG_RFDS:
+	case X86_BUG_SRBDS:
+		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);
+	/*
+	 * GDS can potentially leak data across address spaces and
+	 * threads.  Mitigation is required under all attack vectors.
+	 */
+	case X86_BUG_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) ||
+			cpu_mitigate_attack_vector(CPU_MITIGATE_CROSS_THREAD);
+	default:
+		return false;
+	}
+}
+
+
 /* Default mitigation for MDS-affected CPUs */
 static enum mds_mitigations mds_mitigation __ro_after_init =
 	IS_ENABLED(CONFIG_MITIGATION_MDS) ? MDS_MITIGATION_AUTO : MDS_MITIGATION_OFF;
-- 
2.34.1

On Wed, Jan 08, 2025 at 02:25:01PM -0600, David Kaplan wrote:
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 88eba8e4c7fb..175dbbf9b06e 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -347,6 +347,75 @@ static void x86_amd_ssb_disable(void)
>  		wrmsrl(MSR_AMD64_LS_CFG, msrval);
>  }
>  
> +/*
> + * Returns true if vulnerability should be mitigated based on the
> + * selected attack vector controls

This needs a period.

> + *
> + * See Documentation/admin-guide/hw-vuln/attack_vector_controls.rst
> + */
> +static bool __init should_mitigate_vuln(unsigned int bug)
> +{
> +	switch (bug) {
> +	/*
> +	 * The only spectre_v1 mitigations in the kernel are related to
> +	 * SWAPGS protection on kernel entry.  Therefore, protection is
> +	 * only required for the user->kernel attack vector.
> +	 */

This comment isn't quite correct, there are things like
array_index_nospec() and barrier_nospec() being used, but those aren't
being controlled by bugs.c.  They should at least be mentioned here.

> +	case X86_BUG_SPECTRE_V1:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL);
> +
> +	/*
> +	 * Both spectre_v2 and srso may allow user->kernel or
> +	 * guest->host attacks through branch predictor manipulation.
> +	 */

I don't think this comment adds anything, the code already makes this
obvious.

> +	case X86_BUG_SPECTRE_V2:
> +	case X86_BUG_SRSO:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);

This needs aligned:

		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL) ||
		       cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);

Also, aren't cross-thread attacks possible here, thus the need for
STIBP?  More questions about the cross-thread "vector" below, at the
bottom.

> +	/*
> +	 * spectre_v2_user refers to user->user or guest->guest branch
> +	 * predictor attacks only.  Other indirect branch predictor attacks
> +	 * are covered by the spectre_v2 vulnerability.
> +	 */

The code is already self-evident IMO, I don't think the comment adds
anything.

> +	case X86_BUG_SPECTRE_V2_USER:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);

Another alignment issue.

> +
> +	/* L1TF is only possible as a guest->host attack */

That's not quite correct, PTE inversion is also done to protect against
the user->kernel vector.

Also, IIRC the full l1tf mitigation requires disabling SMT, does that
not qualify as CPU_MITIGATE_CROSS_THREAD?

> +	case X86_BUG_L1TF:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
> +
> +	/*
> +	 * All the vulnerabilities below allow potentially leaking data
> +	 * across address spaces.  Therefore, mitigation is required for
> +	 * any of these 4 attack vectors.
> +	 */
> +	case X86_BUG_MDS:
> +	case X86_BUG_TAA:
> +	case X86_BUG_MMIO_STALE_DATA:
> +	case X86_BUG_RFDS:
> +	case X86_BUG_SRBDS:
> +		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);

Some of these also require disabling SMT for their complete mitigations?

> +	/*
> +	 * GDS can potentially leak data across address spaces and
> +	 * threads.  Mitigation is required under all attack vectors.
> +	 */
> +	case X86_BUG_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) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_CROSS_THREAD);

I'm confused by CPU_MITIGATE_CROSS_THREAD here, as the GDS mitigation
doesn't seem to disable SMT?

Am I just completely misunderstanding the meaning of
CPU_MITIGATE_CROSS_THREAD?

I assumed it's not a vector per se, but rather it means to force nosmt
if one of the other enabled mitigations requires doing so for its "full"
mitigation.  But the implementation doesn't seem to match that.

On the other hand if it really is considered to be its own vector, that
doesn't make sense either, as "cross-thread attack" is really a subset
of each of the other vectors.  For example, a user->kernel attack can
often be done either via syscall/irq or via cross-thread.

So I'm really confused.  Am I missing something?

-- 
Josh

On Tue, Feb 11, 2025 at 10:41:33AM -0800, Josh Poimboeuf wrote:
> I'm confused by CPU_MITIGATE_CROSS_THREAD here, as the GDS mitigation
> doesn't seem to disable SMT?
> 
> Am I just completely misunderstanding the meaning of
> CPU_MITIGATE_CROSS_THREAD?
> 
> I assumed it's not a vector per se, but rather it means to force nosmt
> if one of the other enabled mitigations requires doing so for its "full"
> mitigation.  But the implementation doesn't seem to match that.
> 
> On the other hand if it really is considered to be its own vector, that
> doesn't make sense either, as "cross-thread attack" is really a subset
> of each of the other vectors.  For example, a user->kernel attack can
> often be done either via syscall/irq or via cross-thread.
> 
> So I'm really confused.  Am I missing something?

So I looked at the next patch and now I see what I was missing: the
individual mitigations are checking
cpu_mitigate_attack_vector(CPU_MITIGATE_CROSS_THREAD) before deciding
whether to disable SMT.  So the implementation mostly makes sense now.

should_mitigate_vuln() should have a comment at the top explaining that
it doesn't check CPU_MITIGATE_CROSS_THREAD (since it's not actually a
standalone vector but rather dependent on the others) and that each
individual mitigation should check CPU_MITIGATE_CROSS_THREAD when
deciding whether to disable SMT.

Also, checking CPU_MITIGATE_CROSS_THREAD for GDS doesn't make sense
because as I mentioned above, "cross-thread" is really a subset of the
other vectors.  If the user isn't concerned about any of the other
attack vectors, mitigate_cross_thread=on should just be ignored.

I'm also thinking that "mitigate_cross_thread" isn't quite the right
name for it, as it really only relates to disabling SMT rather than
other cross-thread mitigations like STIBP.

So "mitigate_disable_smt" or "mitigate_nosmt"?

-- 
Josh

On Wed, Jan 08, 2025 at 02:25:01PM -0600, David Kaplan wrote:
> The function should_mitigate_vuln() defines which vulnerabilities should
> be mitigated based on the selected attack vector controls.  The
> selections here are based on the individual characteristics of each
> vulnerability.
> 
> Signed-off-by: David Kaplan <david.kaplan@amd.com>
> ---
>  arch/x86/kernel/cpu/bugs.c | 69 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 88eba8e4c7fb..175dbbf9b06e 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -347,6 +347,75 @@ static void x86_amd_ssb_disable(void)
>  		wrmsrl(MSR_AMD64_LS_CFG, msrval);
>  }
>  
> +/*
> + * Returns true if vulnerability should be mitigated based on the
> + * selected attack vector controls
> + *
> + * See Documentation/admin-guide/hw-vuln/attack_vector_controls.rst
> + */
> +static bool __init should_mitigate_vuln(unsigned int bug)
> +{
> +	switch (bug) {
> +	/*
> +	 * The only spectre_v1 mitigations in the kernel are related to
> +	 * SWAPGS protection on kernel entry.  Therefore, protection is
> +	 * only required for the user->kernel attack vector.
> +	 */
> +	case X86_BUG_SPECTRE_V1:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL);
> +
> +	/*
> +	 * Both spectre_v2 and srso may allow user->kernel or
> +	 * guest->host attacks through branch predictor manipulation.
> +	 */
> +	case X86_BUG_SPECTRE_V2:
> +	case X86_BUG_SRSO:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
> +
> +	/*
> +	 * spectre_v2_user refers to user->user or guest->guest branch
> +	 * predictor attacks only.  Other indirect branch predictor attacks
> +	 * are covered by the spectre_v2 vulnerability.
> +	 */
> +	case X86_BUG_SPECTRE_V2_USER:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);
> +
> +	/* L1TF is only possible as a guest->host attack */
> +	case X86_BUG_L1TF:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
> +
> +	/*
> +	 * All the vulnerabilities below allow potentially leaking data
> +	 * across address spaces.  Therefore, mitigation is required for
> +	 * any of these 4 attack vectors.
> +	 */
> +	case X86_BUG_MDS:
> +	case X86_BUG_TAA:
> +	case X86_BUG_MMIO_STALE_DATA:
> +	case X86_BUG_RFDS:
> +	case X86_BUG_SRBDS:
> +		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);
> +	/*
> +	 * GDS can potentially leak data across address spaces and
> +	 * threads.  Mitigation is required under all attack vectors.
> +	 */
> +	case X86_BUG_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) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_CROSS_THREAD);
> +	default:
> +		return false;

It is missing the case X86_BUG_RETBLEED. should_mitigate_vuln() will always
return false for retbleed.

I am wondering if this function should return true in the default case. So
that in future if someone misses to add a case for a new bug, it will still
be mitigated.