Re: [PATCH v3 20/35] x86/bugs: Define attack vectors

[Josh Poimboeuf]

On Wed, Jan 08, 2025 at 02:25:00PM -0600, David Kaplan wrote:
> Define 5 new attack vectors that are used for controlling CPU
> speculation mitigations and associated command line options.  Each
> attack vector may be enabled or disabled, which affects the CPU
> mitigations enabled.
> 
> The default settings for these attack vectors are consistent with
> existing kernel defaults, other than the automatic disabling of VM-based
> attack vectors if KVM support is not present.
> 
> Signed-off-by: David Kaplan <david.kaplan@xxxxxxx>
> ---
>  arch/x86/include/asm/bugs.h | 11 +++++++
>  arch/x86/kernel/cpu/bugs.c  | 60 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+)
> 
> diff --git a/arch/x86/include/asm/bugs.h b/arch/x86/include/asm/bugs.h
> index f25ca2d709d4..354d04a964f0 100644
> --- a/arch/x86/include/asm/bugs.h
> +++ b/arch/x86/include/asm/bugs.h
> @@ -12,4 +12,15 @@ static inline int ppro_with_ram_bug(void) { return 0; }
>  
>  extern void cpu_bugs_smt_update(void);
>  
> +enum cpu_attack_vectors {
> +	CPU_MITIGATE_USER_KERNEL,
> +	CPU_MITIGATE_USER_USER,
> +	CPU_MITIGATE_GUEST_HOST,
> +	CPU_MITIGATE_GUEST_GUEST,
> +	CPU_MITIGATE_CROSS_THREAD,
> +	NR_CPU_ATTACK_VECTORS,
> +};
> +
> +bool cpu_mitigate_attack_vector(enum cpu_attack_vectors v);
> +
>  #endif /* _ASM_X86_BUGS_H */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index aee2945bdef9..88eba8e4c7fb 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -169,6 +169,66 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
>  DEFINE_STATIC_KEY_FALSE(mmio_stale_data_clear);
>  EXPORT_SYMBOL_GPL(mmio_stale_data_clear);
>  
> +#ifdef CONFIG_CPU_MITIGATIONS
> +/*
> + * All except the cross-thread attack vector are mitigated by default.
> + * Cross-thread mitigation often requires disabling SMT which is too expensive
> + * to be enabled by default.
> + *
> + * Guest-to-Host and Guest-to-Guest vectors are only needed if KVM support is
> + * present.
> + */
> +static bool cpu_mitigate_attack_vectors[NR_CPU_ATTACK_VECTORS] __ro_after_init = {
> +	[CPU_MITIGATE_USER_KERNEL] = true,
> +	[CPU_MITIGATE_USER_USER] = true,
> +	[CPU_MITIGATE_GUEST_HOST] = IS_ENABLED(CONFIG_KVM),
> +	[CPU_MITIGATE_GUEST_GUEST] = IS_ENABLED(CONFIG_KVM),
> +	[CPU_MITIGATE_CROSS_THREAD] = false
> +};
> +
> +#define DEFINE_ATTACK_VECTOR(opt, v) \

s/opt/name/ to distinguish it from v.

> +	static int __init v##_parse_cmdline(char *arg) \

Instead of "CPU_MITIGATE_USER_KERNEL_parse_cmdline" it should really be
"mitigate_user_kernel_cmdline".

Also this line shouldn't be indented.

Also it's more readable to tab align all the line continuation
backslashes.

> +{ \
> +	if (!strcmp(arg, "off")) \
> +		cpu_mitigate_attack_vectors[v] = false; \
> +	else if (!strcmp(arg, "on")) \
> +		cpu_mitigate_attack_vectors[v] = true; \
> +	else \
> +		pr_warn("Unsupported " opt "=%s\n", arg); \
> +	return 0; \
> +} \
> +early_param(opt, v##_parse_cmdline)
> +
> +bool cpu_mitigate_attack_vector(enum cpu_attack_vectors v)
> +{
> +	if (v < NR_CPU_ATTACK_VECTORS)
> +		return cpu_mitigate_attack_vectors[v];
> +
> +	WARN_ON_ONCE(v >= NR_CPU_ATTACK_VECTORS);
> +	return false;
> +}

This error can be checked at build time.

> +#else

This needs a /* !CONFIG_CPU_MITIGATIONS */ comment.

> #endif

As does this.


So, something like so:

#ifdef CONFIG_CPU_MITIGATIONS
/*
 * All except the cross-thread attack vector are mitigated by default.
 * Cross-thread mitigation often requires disabling SMT which is too expensive
 * to be enabled by default.
 *
 * Guest-to-Host and Guest-to-Guest vectors are only needed if KVM support is
 * present.
 */
static bool cpu_mitigate_attack_vectors[NR_CPU_ATTACK_VECTORS] __ro_after_init = {
	[CPU_MITIGATE_USER_KERNEL] = true,
	[CPU_MITIGATE_USER_USER] = true,
	[CPU_MITIGATE_GUEST_HOST] = IS_ENABLED(CONFIG_KVM),
	[CPU_MITIGATE_GUEST_GUEST] = IS_ENABLED(CONFIG_KVM),
	[CPU_MITIGATE_CROSS_THREAD] = false
};

#define DEFINE_ATTACK_VECTOR(name, v)					\
static int __init name##_parse_cmdline(char *arg)			\
{									\
	if (!strcmp(arg, "off"))					\
		cpu_mitigate_attack_vectors[v] = false;			\
	else if (!strcmp(arg, "on"))					\
		cpu_mitigate_attack_vectors[v] = true;			\
	else								\
		pr_warn("Unsupported " __stringify(name) "=%s\n", arg);	\
	return 0;							\
}									\
early_param(__stringify(name), name##_parse_cmdline)

#define cpu_mitigate_attack_vector(v)					\
({									\
	BUILD_BUG_ON(v >= NR_CPU_ATTACK_VECTORS);			\
	cpu_mitigate_attack_vectors[v];					\
})

#else /* !CONFIG_CPU_MITIGATIONS */

#define DEFINE_ATTACK_VECTOR(name, v)					\
static int __init name##_parse_cmdline(char *arg)			\
{									\
	pr_crit("Kernel compiled without mitigations, ignoring %s; system may still be vulnerable\n", \
		__stringify(name));					\
	return 0;							\
}									\
early_param(__stringify(name), name##_parse_cmdline)

#define cpu_mitigate_attack_vector(v) false

#endif /* !CONFIG_CPU_MITIGATIONS */

DEFINE_ATTACK_VECTOR(mitigate_user_kernel, CPU_MITIGATE_USER_KERNEL);
DEFINE_ATTACK_VECTOR(mitigate_user_user, CPU_MITIGATE_USER_USER);
DEFINE_ATTACK_VECTOR(mitigate_guest_host, CPU_MITIGATE_GUEST_HOST);
DEFINE_ATTACK_VECTOR(mitigate_guest_guest, CPU_MITIGATE_GUEST_GUEST);
DEFINE_ATTACK_VECTOR(mitigate_cross_thread, CPU_MITIGATE_CROSS_THREAD);