RE: [PATCH v3 21/35] x86/bugs: Determine relevant vulnerabilities based on attack vector controls.

Kaplan, David
2025-02-11T19:04:47Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> Sent: Tuesday, February 11, 2025 12:54 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 21/35] x86/bugs: Determine relevant vulnerabilities based
> on attack vector controls.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Tue, Feb 11, 2025 at 10:41:33AM -0800, Josh Poimboeuf wrote:
> > 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?
>
> So I looked at the next patch and now I see what I was missing: the individual
> mitigations are checking
> cpu_mitigate_attack_vector(CPU_MITIGATE_CROSS_THREAD) before deciding
> whether to disable SMT. So the implementation mostly makes sense now.
>
> should_mitigate_vuln() should have a comment at the top explaining that it doesn't
> check CPU_MITIGATE_CROSS_THREAD (since it's not actually a standalone
> vector but rather dependent on the others) and that each individual mitigation should
> check CPU_MITIGATE_CROSS_THREAD when deciding whether to disable SMT.
>
> Also, checking CPU_MITIGATE_CROSS_THREAD for GDS doesn't make sense
> because as I mentioned above, "cross-thread" is really a subset of the other
> vectors. If the user isn't concerned about any of the other attack vectors,
> mitigate_cross_thread=on should just be ignored.
>
> I'm also thinking that "mitigate_cross_thread" isn't quite the right name for it, as it
> really only relates to disabling SMT rather than other cross-thread mitigations like
> STIBP.
>
> So "mitigate_disable_smt" or "mitigate_nosmt"?

I'm glad the next patch helped, I can certainly add a comment to help. As you noted, should_mitigate_vuln() is not the only function that determines mitigations.

To explain my thinking a bit more, mitigate_cross_thread is intended to enable cross-thread mitigations for any vulnerabilities the hardware may have. That does not necessarily require disabling SMT. The required cross-thread mitigation is defined by each vulnerability.

For many vulnerabilities (like MDS), mitigation requires disabling SMT. mds_apply_mitigation() queries the status of the cross-thread attack vector and will disable SMT if needed.

For GDS, mitigating cross-thread attacks does not require disabling SMT, just enabling the mitigation in the MSR.

To be fair, it doesn't make much sense to disable all the attack vectors except mitigate_cross_thread, but for correctness it seemed like enabling the mitigation in this case was the right thing.

I don't really want to tie mitigate_cross_thread to SMT disable because of cases like this where there is a cross-thread attack mitigation that is different from disabling SMT. You could also imagine bugs that might be even more limited, where perhaps they're only relevant for say user->kernel but also have a cross-thread component.

STIBP is another case where there is a cross-thread mitigation that is not disabling SMT (but this one is harder to deal with given the historical precedent as noted in the other email). But the point is that there have been cases where cross-thread mitigation != disable SMT.

--David Kaplan