linux-kernel - Re: [PATCH v3 21/35] x86/bugs: Determine relevant vulnerabilities based on attack vector controls.

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250211184130.2fl4jvdwd4e5y32a@jpoimboe>
Date: Tue, 11 Feb 2025 10:41:30 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: David Kaplan <david.kaplan@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.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 v3 21/35] x86/bugs: Determine relevant vulnerabilities
 based on attack vector controls.

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