| Message ID | 20220130211838.8382-4-rick.p.edgecombe@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Shadow stacks for userspace | expand |
On 1/30/22 13:18, Rick Edgecombe wrote: > --- a/arch/x86/kernel/cpu/cpuid-deps.c > +++ b/arch/x86/kernel/cpu/cpuid-deps.c > @@ -78,6 +78,7 @@ static const struct cpuid_dep cpuid_deps[] = { > { X86_FEATURE_XFD, X86_FEATURE_XSAVES }, > { X86_FEATURE_XFD, X86_FEATURE_XGETBV1 }, > { X86_FEATURE_AMX_TILE, X86_FEATURE_XFD }, > + { X86_FEATURE_SHSTK, X86_FEATURE_XSAVES }, > {} > }; Please add a chunk to the changelog that explains the dependency. This would suffice: To protect shadow stack state from malicious modification, the registers are only accessible in supervisor mode. This implementation context-switches the registers with XSAVES. Make X86_FEATURE_SHSTK depend on XSAVES. The XSAVES dependency is touched on in the documentation, but it's a bit buried in there.
On Mon, 2022-02-07 at 14:45 -0800, Dave Hansen wrote: > Please add a chunk to the changelog that explains the dependency. > This > would suffice: > > To protect shadow stack state from malicious modification, > the > registers are only accessible in supervisor mode. This > implementation context-switches the registers with XSAVES. > Make > X86_FEATURE_SHSTK depend on XSAVES. Thanks. Yea, I don't think that part of the design is really elaborated on anywhere. It can be some foreshadowing for the signal stuff later too.
On Sun, Jan 30, 2022 at 01:18:06PM -0800, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Add CPU feature flags for Control-flow Enforcement Technology (CET). > > CPUID.(EAX=7,ECX=0):ECX[bit 7] Shadow stack > CPUID.(EAX=7,ECX=0):EDX[bit 20] Indirect Branch Tracking It looks like this only adds the SHSTK bit, maybe drop mention of IBT here. I wonder if we could land this (and the IBT part) without waiting for everything else in the respective series? -Kees > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Cc: Kees Cook <keescook@chromium.org> > --- > > v1: > - Remove IBT, can be added in a follow on IBT series. > > Yu-cheng v25: > - Make X86_FEATURE_IBT depend on X86_FEATURE_SHSTK. > > Yu-cheng v24: > - Update for splitting CONFIG_X86_CET to CONFIG_X86_SHADOW_STACK and > CONFIG_X86_IBT. > - Move DISABLE_IBT definition to the IBT series. > > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/disabled-features.h | 8 +++++++- > arch/x86/kernel/cpu/cpuid-deps.c | 1 + > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h > index 6db4e2932b3d..c3eb94b13fef 100644 > --- a/arch/x86/include/asm/cpufeatures.h > +++ b/arch/x86/include/asm/cpufeatures.h > @@ -355,6 +355,7 @@ > #define X86_FEATURE_OSPKE (16*32+ 4) /* OS Protection Keys Enable */ > #define X86_FEATURE_WAITPKG (16*32+ 5) /* UMONITOR/UMWAIT/TPAUSE Instructions */ > #define X86_FEATURE_AVX512_VBMI2 (16*32+ 6) /* Additional AVX512 Vector Bit Manipulation Instructions */ > +#define X86_FEATURE_SHSTK (16*32+ 7) /* Shadow Stack */ > #define X86_FEATURE_GFNI (16*32+ 8) /* Galois Field New Instructions */ > #define X86_FEATURE_VAES (16*32+ 9) /* Vector AES */ > #define X86_FEATURE_VPCLMULQDQ (16*32+10) /* Carry-Less Multiplication Double Quadword */ > diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h > index 8f28fafa98b3..b7728f7afb2b 100644 > --- a/arch/x86/include/asm/disabled-features.h > +++ b/arch/x86/include/asm/disabled-features.h > @@ -65,6 +65,12 @@ > # define DISABLE_SGX (1 << (X86_FEATURE_SGX & 31)) > #endif > > +#ifdef CONFIG_X86_SHADOW_STACK > +#define DISABLE_SHSTK 0 > +#else > +#define DISABLE_SHSTK (1 << (X86_FEATURE_SHSTK & 31)) > +#endif > + > /* > * Make sure to add features to the correct mask > */ > @@ -85,7 +91,7 @@ > #define DISABLED_MASK14 0 > #define DISABLED_MASK15 0 > #define DISABLED_MASK16 (DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP| \ > - DISABLE_ENQCMD) > + DISABLE_ENQCMD|DISABLE_SHSTK) > #define DISABLED_MASK17 0 > #define DISABLED_MASK18 0 > #define DISABLED_MASK19 0 > diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c > index c881bcafba7d..bf1b55a1ba21 100644 > --- a/arch/x86/kernel/cpu/cpuid-deps.c > +++ b/arch/x86/kernel/cpu/cpuid-deps.c > @@ -78,6 +78,7 @@ static const struct cpuid_dep cpuid_deps[] = { > { X86_FEATURE_XFD, X86_FEATURE_XSAVES }, > { X86_FEATURE_XFD, X86_FEATURE_XGETBV1 }, > { X86_FEATURE_AMX_TILE, X86_FEATURE_XFD }, > + { X86_FEATURE_SHSTK, X86_FEATURE_XSAVES }, > {} > }; > > -- > 2.17.1 >
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 6db4e2932b3d..c3eb94b13fef 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -355,6 +355,7 @@ #define X86_FEATURE_OSPKE (16*32+ 4) /* OS Protection Keys Enable */ #define X86_FEATURE_WAITPKG (16*32+ 5) /* UMONITOR/UMWAIT/TPAUSE Instructions */ #define X86_FEATURE_AVX512_VBMI2 (16*32+ 6) /* Additional AVX512 Vector Bit Manipulation Instructions */ +#define X86_FEATURE_SHSTK (16*32+ 7) /* Shadow Stack */ #define X86_FEATURE_GFNI (16*32+ 8) /* Galois Field New Instructions */ #define X86_FEATURE_VAES (16*32+ 9) /* Vector AES */ #define X86_FEATURE_VPCLMULQDQ (16*32+10) /* Carry-Less Multiplication Double Quadword */ diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h index 8f28fafa98b3..b7728f7afb2b 100644 --- a/arch/x86/include/asm/disabled-features.h +++ b/arch/x86/include/asm/disabled-features.h @@ -65,6 +65,12 @@ # define DISABLE_SGX (1 << (X86_FEATURE_SGX & 31)) #endif +#ifdef CONFIG_X86_SHADOW_STACK +#define DISABLE_SHSTK 0 +#else +#define DISABLE_SHSTK (1 << (X86_FEATURE_SHSTK & 31)) +#endif + /* * Make sure to add features to the correct mask */ @@ -85,7 +91,7 @@ #define DISABLED_MASK14 0 #define DISABLED_MASK15 0 #define DISABLED_MASK16 (DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP| \ - DISABLE_ENQCMD) + DISABLE_ENQCMD|DISABLE_SHSTK) #define DISABLED_MASK17 0 #define DISABLED_MASK18 0 #define DISABLED_MASK19 0 diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c index c881bcafba7d..bf1b55a1ba21 100644 --- a/arch/x86/kernel/cpu/cpuid-deps.c +++ b/arch/x86/kernel/cpu/cpuid-deps.c @@ -78,6 +78,7 @@ static const struct cpuid_dep cpuid_deps[] = { { X86_FEATURE_XFD, X86_FEATURE_XSAVES }, { X86_FEATURE_XFD, X86_FEATURE_XGETBV1 }, { X86_FEATURE_AMX_TILE, X86_FEATURE_XFD }, + { X86_FEATURE_SHSTK, X86_FEATURE_XSAVES }, {} };