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

Kaplan, David
2025-02-17T20:38:10Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> Sent: Monday, February 17, 2025 2:19 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 Mon, Feb 17, 2025 at 05:33:24PM +0000, Kaplan, David wrote:
> > 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?
>
> Hm. We *could* put the cpu_mitigate_attack_vector() macro in bugs.h and make the
> array global (and possibly exported). That way anybody could call it, but it would still
> have the compile-time check.
>
>
> However... should these not actually be arch-generic options, like mitigations=
> already is? It would make for a more consistent user interface across arches.

That's what I had in my patch series up until this one. Boris said to move them to x86-specific code because nobody else is using them yet and somebody down the road could move them.

I do agree that they can be arch-generic (hence why I originally put them in kernel/cpu.c) but I also don't know when (if ever) anyone from other archs will want to pick them up.

>
> They could even be integrated into the "mitigations=" interface. The options could
> be combined in any order (separated by commas):
>
> mitigations=user_kernel,user_user
> mitigations=guest_host,user_kernel
> ...etc...
>
> And e.g., "mitigations=off" would simply disable all the vectors.

Hmm, that's an interesting idea. I assume that any vectors not listed would be considered 'off', unless no mitigations= was specified, or mitigations=auto was specified in which case they'd default to 'on' like they do today.

In other words:
mitigations=auto
=> all 4 vectors are 'on'
mitigations=user_kernel
=> user_kernel is 'on', all others are 'off'

That would also imply that:
mitigations=user_kernel mitigations=user_user

Would actually mean that user_user is 'on' and everything is 'off'. Not sure if that's an issue or would be sufficiently obvious.

Then a question is how to integrate the mitigate_smt option we were just discussing since that needs a 3-way select. Or perhaps keep that one as a separate command line option.

>
> That would remove ambiguity created by combining mitigations= with
> mitigate_* and would make it easier for all the current
> cpu_mitigations_off() callers: only one global enable/disable interface to call instead
> of two. Any code calling cpu_mitigations_off() should probably be calling something
> like cpu_mitigate_attack_vector() instead.
>
> cpu_mitigations_off() and cpu_mitigations_auto_nosmt() could be deprecated in
> favor of more vector-specific interfaces, and could be removed once all the arches
> stop using them. They could be gated by a temporary
> ARCH_USES_MITIGATION_VECTORS option. As could the per-vector cmdline
> options.
>
> Thoughts?
>

I'm not sure there is really that much ambiguity...the global mitigations=off is the big button that disables everything. I don't think we can change that.

I think the other issue here may be that the attack vectors are defined to be rather low-priority in terms of selection. That is, you can disable all the attack vectors but then still enable an individual bug fix.

In other words, if you were to replace cpu_mitigations_off() with a function looking for all attack vectors to be off, that isn't quite correct because of the priority difference.

--David Kaplan