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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250227003528.hnviwrtzs7jc3juj@desk>
Date: Wed, 26 Feb 2025 16:35:28 -0800
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: "Kaplan, David" <David.Kaplan@....com>, Borislav Petkov <bp@...en8.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"x86@...nel.org" <x86@...nel.org>,
	"H . Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 20/35] x86/bugs: Define attack vectors

On Wed, Feb 26, 2025 at 03:44:40PM -0800, Josh Poimboeuf wrote:
> On Wed, Feb 26, 2025 at 02:13:24PM -0800, Pawan Gupta wrote:
> > On Wed, Feb 26, 2025 at 09:03:58PM +0000, Kaplan, David wrote:
> > > > Extending =auto to take attack vectors is going to be tricky, because it already
> > > > takes ",nosmt" which would conflict with ",no_cross_thread".
> > > >
> > > > How about we keep =off, and =auto as is, and add:
> > > >
> > > >   mitigations=selective,no_user_kernel,no_cross_thread,...
> > > >
> > > > Requiring the user to explicitly select attack vectors to disable (rather than to
> > > > enable). This would be more verbose, but it would be clear that the user is explicitly
> > > > selecting attack vectors to disable. Also, if a new attack vector gets added in future,
> > > > it would be mitigated by default, without requiring the world to change their cmdline.
> > > 
> > > I kind of like that.
> 
> While it might be true that we don't necessarily need both opt-in and
> opt-out options...
> 
> I'm missing the point of the "selective" thing vs just
> "auto,no_whatever"?

That was my first thought, but then I realized that in "auto,nosmt" nosmt
is the opposite of disabling the mitigation. It would be cleaner to have
"=selective,no_whatever" which is self-explanatory.

> > > Note that for the SMT stuff, my new plan had been to use a separate
> > > option 'mitigate_smt' which will be on/off/auto.
> >
> > I would avoid that, because we can't drop support for
> > "mitigations=auto,nosmt"
> 
> We wouldn't have to drop support for that...  If there's a conflict
> between the two options then just print a warning and pick one.

Introducing one more option for smt seems unnecessary. We already have
auto,nosmt and nosmt. Trying to guess which one takes precedence would
be confusing.

> > and we also have a separate cmdline parameter
> > "nosmt":
> > 
> > 	nosmt		[KNL,MIPS,PPC,S390,EARLY] Disable symmetric multithreading (SMT).
> > 			Equivalent to smt=1.
> > 
> > 			[KNL,X86,PPC] Disable symmetric multithreading (SMT).
> > 			nosmt=force: Force disable SMT, cannot be undone
> > 				     via the sysfs control file.
> 
> The separate 'nosmt' option is orthogonal to the mitigation stuff.  If
> it disables SMT then there are no cross-thread mitigations to do in the
> first place.

Right.

> > > But we could also combine that with mitigations=selective perhaps with
> > > tokens like 'mitigate_smt' (enable all relevant SMT mitigations including
> > > disabling SMT if needed) or 'no_mitigate_smt' (do not enable any SMT
> > > mitigation).  If no token is specified, then we'd default to the behavior
> > > today where SMT won't be disabled but other mitigations get applied.
> > > Then everything is in one option.
> > 
> > Agree.
> 
> I think that's *way* too subtle.  It's completely unlike the other
> options in that it's not a binary opt-out.  And it sneakily obfuscates
> the mitigate_smt tristate (with the third state being the unspecified
> default).
> 
> However, one of those three states is already represented by
> 'auto,nosmt'.  Why not just piggyback on that by allowing the vectors to
> be combined with it?  Then we only need two more states, which could be
> represented with e.g., "[no_]cross_thread".
> 
> For example, to disable SMT (if needed), along with disabling of
> vectors:
>   
>   mitigations=auto,nosmt,no_user_kernel,etc
> 
> Or to disable all SMT mitigations (e.g., because the user is doing core
> scheduling):
> 
>   mitigations=auto,no_cross_thread,etc
> 
> And combining 'auto,nosmt' with 'no_cross_thread' is nonsensical, in
> which case so it could just pick the former and print a warning.

That seems reasonable. The only thing is now we are mixing enabling and
disabling mitigations in the attack vector list. And that probably is
better than having a separate parameter.