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

Kaplan, David
2025-02-17T17:33:26Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Kaplan, David
> Sent: Wednesday, February 12, 2025 11:21 AM
> To: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Borislav Petkov <bp@alien8.de>; Peter
> Zijlstra <peterz@infradead.org>; Pawan Gupta
> <pawan.kumar.gupta@linux.intel.com>; Ingo Molnar <mingo@redhat.com>; Dave
> Hansen <dave.hansen@linux.intel.com>; x86@kernel.org; H . Peter Anvin
> <hpa@zytor.com>; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v3 20/35] x86/bugs: Define attack vectors
>
>
>
> > -----Original Message-----
> > From: Josh Poimboeuf <jpoimboe@kernel.org>
> > Sent: Tuesday, February 11, 2025 12:08 PM
> > To: Kaplan, David <David.Kaplan@amd.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>; Borislav Petkov
> > <bp@alien8.de>; Peter Zijlstra <peterz@infradead.org>; Pawan Gupta
> > <pawan.kumar.gupta@linux.intel.com>; Ingo Molnar <mingo@redhat.com>;
> > Dave Hansen <dave.hansen@linux.intel.com>; x86@kernel.org; H . Peter
> > Anvin <hpa@zytor.com>; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 20/35] x86/bugs: Define attack vectors
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > 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@amd.com>
> > > ---
> > > 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);
>

So actually this doesn't quite work because the code in arch/x86/mm/pti.c has to call cpu_mitigate_attack_vector in order to check if PTI is required (it checks if user->kernel mitigations are needed). That's the only use of the attack vectors outside of bugs.c.

The original code (using a function and WARN_ON_ONCE) can work, or I could perhaps create a pti-specific function in bugs.c that the pti code can query. But right now I don't think there is any pti-related code in bugs.c at all.

Any suggestion?

--David Kaplan