RE: [RFC PATCH 07/56] x86/bugs: Reset spectre_v2_user mitigations

Kaplan, David
2025-10-16T15:26:54Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Brendan Jackman <jackmanb@google.com>
> Sent: Thursday, October 16, 2025 9:57 AM
> To: Kaplan, David <David.Kaplan@amd.com>; Brendan Jackman
> <jackmanb@google.com>; Thomas Gleixner <tglx@linutronix.de>; Borislav Petkov
> <bp@alien8.de>; Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@kernel.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>
> Cc: Alexander Graf <graf@amazon.com>; Boris Ostrovsky
> <boris.ostrovsky@oracle.com>; linux-kernel@vger.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@google.com>
> >> Sent: Thursday, October 16, 2025 7:54 AM
> >> To: Kaplan, David <David.Kaplan@amd.com>; Thomas Gleixner
> >> <tglx@linutronix.de>; Borislav Petkov <bp@alien8.de>; Peter Zijlstra
> >> <peterz@infradead.org>; Josh Poimboeuf <jpoimboe@kernel.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>
> >> Cc: Alexander Graf <graf@amazon.com>; Boris Ostrovsky
> >> <boris.ostrovsky@oracle.com>; linux-kernel@vger.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@amd.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.

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).

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?

--David Kaplan