RE: [PATCH] x86/fpu: Disable shstk if no CET_USER state

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> Sent: Friday, April 3, 2026 2:36 PM
> To: Kaplan, David <David.Kaplan@xxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>;
> Borislav Petkov <bp@xxxxxxxxx>; Dave Hansen
> <dave.hansen@xxxxxxxxxxxxxxx>; x86@xxxxxxxxxx; H. Peter Anvin
> <hpa@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] x86/fpu: Disable shstk if no CET_USER state
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On Fri, Apr 03, 2026, David Kaplan wrote:
> > Some hypervisors (including QEMU 10.1.5) may report CET_SS support in
> > CPUID Fn7 but fail to report that CET_USER state is supported in
> > supervisor xstate.  Linux relies on XSAVES/XRSTORS to swap CET state
> > during context switch and assumes it is supported when CET_SS is
> > present.
> >
> > As a result, if a user process is run with shadow stacks enabled and
> > then is switched away from, the system may crash because the new process
> > may be incorrectly run with shadow stacks enabled.
> >
> > Detect this broken configuration and disable user shadow stacks unless
> > CET_USER is supported in xstate.
>
> It's not actually broken though, is it?  Just "odd".  AFAICT, neither the SDM
> nor the APM _requires_ CET_{U,S} to be supported in XSS if shadow stacks
> are
> suppported.

The APM language at least isn't super clear on whether this is legal, but there are no real CPUs to my knowledge that do not enumerate XSS support for these.  But this configuration is definitely broken for Linux, because it will eventually crash with random segfaults (and unfortunately this often takes minutes to manifest since very few processes actually try to run with shadow stacks apparently).

>
> > Signed-off-by: David Kaplan <david.kaplan@xxxxxxx>
> > ---
> >  arch/x86/kernel/fpu/xstate.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index 76153dfb58c9..188323442b4d 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -855,6 +855,17 @@ void __init fpu__init_system_xstate(unsigned int
> legacy_size)
> >               goto out_disable;
> >       }
> >
> > +     if (boot_cpu_has(X86_FEATURE_USER_SHSTK) &&
> > +         !(fpu_kernel_cfg.max_features & XFEATURE_MASK_CET_USER)) {
> > +             /*
> > +              * The kernel relies on XSAVES/XRSTORS to context switch shadow
> > +              * stack state.  If this isn't present, disable user shadow
> > +              * stacks.
> > +              */
> > +             pr_err("x86/fpu: CET_USER not supported in xstate when CET is
> supported.  Disabling shadow stacks.\n");
> > +             setup_clear_cpu_cap(X86_FEATURE_USER_SHSTK);
>
> Doesn't this apply to IBT as well?  This code is also misplaced, as it needs to
> live after at least this code:

Good point, it likely does.  I can't confirm that as I don't have IBT hardware, but assuming that a guest can see CET_IBT=1 this same problem would exist.


>
>         if (!cpu_feature_enabled(X86_FEATURE_XSAVES))
>                 fpu_kernel_cfg.max_features &=
> XFEATURE_MASK_USER_SUPPORTED;
>         else
>                 fpu_kernel_cfg.max_features &=
> XFEATURE_MASK_USER_SUPPORTED |
>                                         XFEATURE_MASK_SUPERVISOR_SUPPORTED;
>
> and should probably play nice with the "out_disable" path too.
>

Ok, that makes sense.  I'm not sure if the out_disable path ever gets used, but wouldn't that cause other problems if XSAVE is fully disabled?  I don't see it clearing any feature bits related to other XSTATE components.

--David Kaplan

> All in all, setup_cet() seems like a much better fit, but unfortunately that
> runs before fpu__init_system() :-(