linux-kernel - RE: [RFC PATCH 04/56] x86/bugs: Reset spectre

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:
 <LV3PR12MB926554015AFB93D35AE3098094EBA@LV3PR12MB9265.namprd12.prod.outlook.com>
Date: Tue, 14 Oct 2025 19:16:27 +0000
From: "Kaplan, David" <David.Kaplan@....com>
To: Dave Hansen <dave.hansen@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>, Peter Zijlstra <peterz@...radead.org>, Josh
 Poimboeuf <jpoimboe@...nel.org>, Pawan Gupta
	<pawan.kumar.gupta@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>, Dave
 Hansen <dave.hansen@...ux.intel.com>, "x86@...nel.org" <x86@...nel.org>, "H .
 Peter Anvin" <hpa@...or.com>
CC: Alexander Graf <graf@...zon.com>, Boris Ostrovsky
	<boris.ostrovsky@...cle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 04/56] x86/bugs: Reset spectre_v1 mitigations

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Dave Hansen <dave.hansen@...el.com>
> Sent: Tuesday, October 14, 2025 1:38 PM
> To: Kaplan, David <David.Kaplan@....com>; Thomas Gleixner
> <tglx@...utronix.de>; Borislav Petkov <bp@...en8.de>; Peter Zijlstra
> <peterz@...radead.org>; Josh Poimboeuf <jpoimboe@...nel.org>; Pawan Gupta
> <pawan.kumar.gupta@...ux.intel.com>; Ingo Molnar <mingo@...hat.com>; Dave
> Hansen <dave.hansen@...ux.intel.com>; x86@...nel.org; H . Peter Anvin
> <hpa@...or.com>
> Cc: Alexander Graf <graf@...zon.com>; Boris Ostrovsky
> <boris.ostrovsky@...cle.com>; linux-kernel@...r.kernel.org
> Subject: Re: [RFC PATCH 04/56] x86/bugs: Reset spectre_v1 mitigations
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 10/13/25 07:33, David Kaplan wrote:
> ...
> > +#ifdef CONFIG_DYNAMIC_MITIGATIONS
> > +static void spectre_v1_reset_mitigation(void)
> > +{
> > +     setup_clear_cpu_cap(X86_FEATURE_FENCE_SWAPGS_USER);
> > +     setup_clear_cpu_cap(X86_FEATURE_FENCE_SWAPGS_KERNEL);
> > +     spectre_v1_mitigation = SPECTRE_V1_MITIGATION_AUTO;
> > +}
> > +#endif
> > +
> >  static int __init nospectre_v1_cmdline(char *str)
> >  {
> >       spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;
> > @@ -3794,3 +3805,10 @@ void __warn_thunk(void)
> >  {
> >       WARN_ONCE(1, "Unpatched return thunk in use. This should not
> happen!\n");
> >  }
> > +
> > +#ifdef CONFIG_DYNAMIC_MITIGATIONS
> > +void arch_cpu_reset_mitigations(void)
> > +{
> > +     spectre_v1_reset_mitigation();
> > +}
> > +#endif
>
> Are all the #ifdefs necessary? For instance, a single:
>
>  void arch_cpu_reset_mitigations(void)
>  {
> +       if (!IS_ENABLED(CONFIG_DYNAMIC_MITIGATIONS))
> +               return;
> ...
>
> and removing *all* the #ifdefs might be enough to tell the compiler that
> all the *_reset_mitigation() functions are unreachable.

Oh, hmm yeah that could be worth trying instead.

>
> But the larger concern through all of this is that this is all *new*
> code. The reset concept is completely new and the reset functions look
> to be ad-hoc. They follow a few patterns, but nothing super consistent.

The reset logic is basically 'undo'ing the apply_mitigation() functions.  So any feature flag, static branch, etc. that may get touched in the apply function are reset in the reset function.

That's not the only way this could be structured...there could be a single function that resets any and all mitigation-related things used by any of the mitigation (there is certainly overlap in many of them).  When writing it, I found the chosen approach to be pretty straightforward because it was easy enough to model the reset function after the apply function (and that's why it typically follows the apply function in the file).

>
> Also, this series is going to need to be broken up. I'd suggest going
> after the dynamic alternatives, first. I'm sure there's a need for that
> _somewhere_ other than for vulnerabilities.

Ok.  I'm certainly open to ideas of what else could benefit from this type of functionality.

I think to properly re-patch vulnerabilities, we need all 3 things (alternatives, returns, indirects).  We don't want to create some intermediate thing where the kernel is partially patched in one way and partially patched in another way.  Part of my goal here was that after re-patching, things should look like you had originally booted under those same options.

But if there was another use case for say just re-patching alternatives, that support could presumably be eventually leveraged to patch mitigations.

>
> But, in the end, (IMNHO) this series really just adds complexity. It
> doesn't do enough refactoring or complexity reduction to go in with this
> approach.

Fwiw, this series does actually help a lot with mitigation testing.  In fact, as part of the development I wrote a fuzzer to test various combinations of X86_BUG bits and command line options.  That found several bugs in upstream in fact (which I submitted separate patches for).  Given the number of different pieces of hardware that exist, various cmdline options (and their interactions), I see being able to better test the mitigation logic as a positive thing.  Obviously that must be balanced against the overall complexity, but I think it's worth noting that there may be some maintainability benefits too of runtime re-configuration.

--David Kaplan