Message-ID: <DDJVO9NSVDOZ.1JS1JRMXOKBB3@google.com>
Date: Thu, 16 Oct 2025 16:13:25 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: "Kaplan, David" <David.Kaplan@....com>, Brendan Jackman <jackmanb@...gle.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 07/56] x86/bugs: Reset spectre_v2_user mitigations

On Thu Oct 16, 2025 at 3:26 PM UTC, David Kaplan wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>> -----Original Message-----
>> From: Brendan Jackman <jackmanb@...gle.com>
>> Sent: Thursday, October 16, 2025 9:57 AM
>> To: Kaplan, David <David.Kaplan@....com>; Brendan Jackman
>> <jackmanb@...gle.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 07/56] x86/bugs: Reset spectre_v2_user mitigations
>>
>> Caution: This message originated from an External Source. Use proper caution
>> when opening attachments, clicking links, or responding.
>>
>>
>> On Thu Oct 16, 2025 at 2:06 PM UTC, David Kaplan wrote:
>> > [AMD Official Use Only - AMD Internal Distribution Only]
>> >
>> >> -----Original Message-----
>> >> From: Brendan Jackman <jackmanb@...gle.com>
>> >> Sent: Thursday, October 16, 2025 7:54 AM
>> >> 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 07/56] x86/bugs: Reset spectre_v2_user mitigations
>> >>
>> >> Caution: This message originated from an External Source. Use proper caution
>> >> when opening attachments, clicking links, or responding.
>> >>
>> >>
>> >> On Mon Oct 13, 2025 at 2:33 PM UTC, David Kaplan wrote:
>> >> > Add function to reset spectre_v2_user mitigations back to their boot-time
>> >> > defaults.
>> >> >
>> >> > Signed-off-by: David Kaplan <david.kaplan@....com>
>> >> > ---
>> >> >  arch/x86/kernel/cpu/bugs.c | 13 +++++++++++++
>> >> >  1 file changed, 13 insertions(+)
>> >> >
>> >> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>> >> > index 1f56ccb5f641..4ca46f58e384 100644
>> >> > --- a/arch/x86/kernel/cpu/bugs.c
>> >> > +++ b/arch/x86/kernel/cpu/bugs.c
>> >> > @@ -2056,6 +2056,18 @@ static void __init
>> >> spectre_v2_user_apply_mitigation(void)
>> >> >       }
>> >> >  }
>> >> >
>> >> > +#ifdef CONFIG_DYNAMIC_MITIGATIONS
>> >> > +static void spectre_v2_user_reset_mitigation(void)
>> >> > +{
>> >> > +     static_branch_disable(&switch_vcpu_ibpb);
>> >> > +     static_branch_disable(&switch_mm_always_ibpb);
>> >> > +     static_branch_disable(&switch_mm_cond_ibpb);
>> >> > +     spectre_v2_user_stibp = SPECTRE_V2_USER_NONE;
>> >> > +     spectre_v2_user_ibpb = SPECTRE_V2_USER_NONE;
>> >> > +     spectre_v2_user_cmd = SPECTRE_V2_USER_CMD_AUTO;
>> >> > +}
>> >> > +#endif
>> >> > +
>> >> >  static const char * const spectre_v2_strings[] = {
>> >> >       [SPECTRE_V2_NONE]                       = "Vulnerable",
>> >> >       [SPECTRE_V2_RETPOLINE]                  = "Mitigation: Retpolines",
>> >> > @@ -3844,5 +3856,6 @@ void arch_cpu_reset_mitigations(void)
>> >> >       spectre_v1_reset_mitigation();
>> >> >       spectre_v2_reset_mitigation();
>> >> >       retbleed_reset_mitigation();
>> >> > +     spectre_v2_user_reset_mitigation();
>> >> >  }
>> >> >  #endif
>> >>
>> >> I think this might be failing to account for task state? E.g. if a
>> >> user boots with spectre_v2=off then we ignore the PR_SPEC_DISABLE calls
>> >> that would enable IBPB-on-context-switch for that task. Then if they
>> >> enable it via this dynamic interface they probably expect their
>> >> PR_SPEC_DISABLE to take effect retroactively. I don't think it will with
>> >> the current code, do I have that right?
>> >
>> > If I'm reading the logic correct, if a process tries to do PR_SPEC_DISABLE say
>> for indirects but spectre_v2_user=off then they'll get -EPERM, so we don't ignore it.
>> >
>> > But there could be a case where spectre_v2=on (aka force), when
>> PR_SPEC_DISABLE does get ignored.  And then if spectre_v2 is changed to
>> something else like prctl then the relevant task flags weren't set.
>>
>> Er yeah good point, my example was wrong but the issue exists to some
>> extent.
>>
>> > Not sure the best way to handle this...even if we were to always set the task flags
>> in this case, there could be other cases where the process might think it could set
>> this flag and then get surprised with an -EPERM.  Open to ideas here.
>>
>> Hm, isn't the issue of surprise-EPERM orthogonal to the task state
>> issue? I suspect I'm doing a bit of motivated reasoning here, but it
>> feels to me like a userspace bug for someone to infer statically whether
>> they should expect -EPERM here. Like, the docs don't say that much, but
>> they do say something about what the prctls do, and that doesn't include
>> anything about the _reason_ you got -EPERM.
>>
>> (BTW, it's not relevant here but I actually think -EPERM is a bug [0])
>>
>> [0] https://lore.kernel.org/all/DDJU0415JEBQ.H2SD942NMDWX@google.com/
>
> Well, if you do PR_GET_SPECULATION_CTRL, and it says PR_SPEC_PRCTL=1 that means you can control it per-task.  But then if you later get an error message when you try to set it, that would seem weird.

Yeah I see, I didn't notice there was a case where we _explicitly_ say
it can be controlled (I was just thinking of -EPERM/-ENXIO). It does
seem weird to suddenly countermand that one.

> I don't know if anyone is actually using these controls today but I am nervous about causing confusion (even if the API is rather unclear).

I do know of some users of this API.

> One potential option here could be to always return PR_SPEC_ENABLE if dynamic mitigations are enabled (telling userspace they cannot control the mitigation and they should assume the speculation is enabled).  Or maybe create new options that allow a process to say 'try to give me this speculation control' but without any promise that they'll actually get it.
>
> Assuming that dynamic mitigations get locked down late in boot as part of kernel lockdown, there may only be a relatively small period of time where there is a risk of things changing underneath a task so maybe telling userspace they can't control these until things are locked is ok?

I don't think this should be designed under the assumption that everyone
is using lockdown. It's correct that lockdown disables this feature but
that doesn't mean lockdown is the right security decision for all users.
It would be a shame if it didn't actually support the use case of "oh
no, we found out our assumptions were wrong, we want to change our CPU
posture without rebooting everything". And I think if we just always
returned PR_SPEC_ENABLE that would essentially just mean you can't have
both dynamic control and efficient per-process control.

For PR_SPEC_ENABLE/DISABLE in the return value of
PR_GET_SPECULATION_CTRL, I think we should just return the current state
accurately. The fact that this can now change below userspace's feet
seems kinda above their pay-grade to me.

For PR_SPEC_PRCTl, I guess we could leave it clear and add a new bit to
say "I'm dynamic, you can try the PRCTL but I might randomly fail"? Then
code that doesn't know about the new bit doesn't get surprise errors, it
just has to do whatever fallbacks it would do on unsupported hardware.
Then new code can be updated to deal with random failures gracefully.

This seems a bit over-complicated though, in practice it seems pretty
likely that just hitting people with the unexpected errors is sort of
OK? At least for the usecase I'm seeing everything through the lens of,
I think it would be. 

Powered by blists - more mailing lists