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

Kaplan, David
2025-02-18T02:24:03Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> Sent: Monday, February 17, 2025 5:39 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 08:38:07PM +0000, Kaplan, David wrote:
> > > 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.
>
> Ah, I guess I missed the previous versions. Sorry :-/
>
> > 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.
>
> Well, for example, we already have the generic
> prctl(PR_SET_SPECULATION_CTRL) which is used by arm64. If somebody boots
> with mitigate_user_user=off on arm64, they could reasonably expect those context
> switch mitigations to be disabled.

I'm not sure which this is an argument for actually :)

That is, I do agree that mitigate_user_user makes sense in an arch-agnostic way and a user might expect context switch mitigations disabled. However this patch series doesn't implement these attack vector controls for anything other than x86. So I guess I'm not sure if your argument is that because they aren't yet implemented for arm64, then keeping them x86-specific is better...or if we should make them generic to more easily support extension to other architectures.

>
> > > 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.
>
> Hm, so I was actually thinking that multiple "mitigations=" options would combine. I
> guess either way is confusing. The problem is that they have side effects. Another
> idea below.

Yeah I'm not a big fan of this either, since I don't think this is consistent with how other mitigation command lines are handled.

>
> > 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.
>
> Yeah, the smt tri-state would need some kind of interface as well.

I'd probably vote for just making that 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.
>
> Well sure, but that's because you already know it's the big button ;-) If you don't
> know that, it's nonobvious.
>
> Joe Admin might assume the cmdline options are processed in order, e.g:
>
> mitigations=off mitigate_user_kernel=on
>
> in which case they might reasonably expect to have only user->kernel mitigations
> enabled. Otherwise there would be no point in combining them in the first place.
>
> How about negative and positive versions of each:
>
> mitigations=[no_]user_kernel
> migitations=[no_]user_user
>
> etc.
>
> And then the mitigations= cmdline could simply be processed in order, without side
> effects, to give the user full flexibility. To opt-in to specific vectors:
>
> mitigations=off mitigations=user_kernel,host_guest

I don't like this idea as much as the next, because as noted above, I think with most other command lines, a later version replaces an earlier one, it doesn't append to it.

That is, something like "spectre_v2=retpoline spectre_v2=ibrs" ends up just meaning ibrs.

>
> which is equivalent to:
>
> mitigations=off,user_kernel,host_guest
>
> Or, if one prefers to opt-out:
>
> mitigations=auto,no_user_user,no_guest_guest
>
> where the "auto" is optional for default configs.

This seems more appealing to me because I think it's clearer what 'on' vs 'off' is. It retains the more compact form of the command line while also allowing for opt-in or opt-out style. And if you specify multiple "mitigations=" command lines, the new one replaces the old one, like with most other options.

So I rather like this, and would be interested in what others think too.

>
> > 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.
>
> Hm. So IIUC, the priority (in descending order) is:
>
> 1) mitigations=
>
> 2) individual mitigations, e.g. spectre_v2=
>
> 3) mitigate_*=
>
> 4) defaults
>
> That seems overly complex and nonobvious, where most of the complexity comes
> from handling rarely (or never?) used edge cases.

Your understanding is correct, although I'd argue there isn't really a case 4, since there are default settings for the attack vectors (case 3) which step in if 1) or 2) do not exist. And case 1 is only 'mitigations=off'.

Having individual mitigations override attack vectors to me makes sense because they're more specific. I think the bigger question is the big mitigations=off hammer.

>
> Does the current "big button" behavior for mitigations=off even make sense? Why
> would somebody do "mitigations=off spectre_v2=eibrs" and expect the spectre_v2
> mitigation to be *disabled*? Do we really think anybody relies on that and gets the
> result they were expecting?

A valid question, but this gets into changing existing behavior and that makes me nervous.

I can't say for certain what people do, but I could potentially imagine a system that has some custom configuration of bug options, and then for testing somebody decides to just append 'mitigations=off' at the end of the command line. Perhaps they might reasonably expect that to disable everything? And that is how it'd work today.

>
> The priority could be simplified:
>
> 1) individual mitigations (=auto means use system-wide default)
>
> 2) system-wide defaults (tweaked by mitigations=/mitigate_*=)
>
> So the system-wide defaults would be defined by mitigations=whatever, and those
> can be overriden by the individual mitigations. That seems a lot more simple and
> logical.
>
> And since you're already introducing "=auto" for the individual mitigations, I think that
> would be easy enough to implement.
>

Yes it wouldn't be very hard, and it does make logical sense. But I think the big caveat is the change to the existing mitigations=off and that it would no longer override individual mitigations. Changing that would seem to be a risk, and is that risk worth taking? It doesn't seem like it's that much harder to implement the 3-tier scheme (mitigations=off, individual, attack-vectors), especially since everything already checks for cpu_mitigations_off().

--David Kaplan