| Message ID | 20221203003606.6838-5-rick.p.edgecombe@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Shadow stacks for userspace | expand |
On Fri, Dec 02, 2022 at 04:35:31PM -0800, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Setting CR4.CET is a prerequisite for utilizing any CET features, most of > which also require setting MSRs. > > Kernel IBT already enables the CET CR4 bit when it detects IBT HW support > and is configured with kernel IBT. However, future patches that enable > userspace shadow stack support will need the bit set as well. So change > the logic to enable it in either case. > > Clear MSR_IA32_U_CET in cet_disable() so that it can't live to see > userspace in a new kexec-ed kernel that has CR4.CET set from kernel IBT. > > Tested-by: Pengfei Xu <pengfei.xu@intel.com> > Tested-by: John Allen <john.allen@amd.com> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> Reviewed-by: Kees Cook <keescook@chromium.org>
On Fri, Dec 02, 2022 at 04:35:31PM -0800, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Setting CR4.CET is a prerequisite for utilizing any CET features, most of > which also require setting MSRs. ... > arch/x86/kernel/cpu/common.c | 37 ++++++++++++++++++++++++++++++------ > 1 file changed, 31 insertions(+), 6 deletions(-) Looks better. Let's get rid of the ifdeffery and simplify it even more. Diff ontop: --- diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 579f10222432..c364f3067121 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -597,12 +597,14 @@ __noendbr void ibt_restore(u64 save) #endif -#ifdef CONFIG_X86_CET static __always_inline void setup_cet(struct cpuinfo_x86 *c) { - bool kernel_ibt = HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT); - bool user_shstk; - u64 msr = 0; + bool kernel_ibt, user_shstk; + + if (!IS_ENABLED(CONFIG_X86_CET)) + return; + + kernel_ibt = HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT); /* * Enable user shadow stack only if the Linux defined user shadow stack @@ -618,21 +620,18 @@ static __always_inline void setup_cet(struct cpuinfo_x86 *c) set_cpu_cap(c, X86_FEATURE_USER_SHSTK); if (kernel_ibt) - msr = CET_ENDBR_EN; + wrmsrl(MSR_IA32_S_CET, CET_ENDBR_EN); + else + wrmsrl(MSR_IA32_S_CET, 0); - wrmsrl(MSR_IA32_S_CET, msr); cr4_set_bits(X86_CR4_CET); if (kernel_ibt && !ibt_selftest()) { pr_err("IBT selftest: Failed!\n"); wrmsrl(MSR_IA32_S_CET, 0); setup_clear_cpu_cap(X86_FEATURE_IBT); - return; } } -#else /* CONFIG_X86_CET */ -static inline void setup_cet(struct cpuinfo_x86 *c) {} -#endif __noendbr void cet_disable(void) {
On Wed, 2022-12-07 at 13:49 +0100, Borislav Petkov wrote: > On Fri, Dec 02, 2022 at 04:35:31PM -0800, Rick Edgecombe wrote: > > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > > > Setting CR4.CET is a prerequisite for utilizing any CET features, > > most of > > which also require setting MSRs. > > ... > > > arch/x86/kernel/cpu/common.c | 37 ++++++++++++++++++++++++++++++- > > ----- > > 1 file changed, 31 insertions(+), 6 deletions(-) > > Looks better. > > Let's get rid of the ifdeffery and simplify it even more. Diff ontop: Sure, thanks!
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 73cc546e024d..579f10222432 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -597,29 +597,51 @@ __noendbr void ibt_restore(u64 save) #endif +#ifdef CONFIG_X86_CET static __always_inline void setup_cet(struct cpuinfo_x86 *c) { - u64 msr = CET_ENDBR_EN; + bool kernel_ibt = HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT); + bool user_shstk; + u64 msr = 0; + + /* + * Enable user shadow stack only if the Linux defined user shadow stack + * cap was not cleared by command line. + */ + user_shstk = cpu_feature_enabled(X86_FEATURE_SHSTK) && + IS_ENABLED(CONFIG_X86_USER_SHADOW_STACK); - if (!HAS_KERNEL_IBT || - !cpu_feature_enabled(X86_FEATURE_IBT)) + if (!kernel_ibt && !user_shstk) return; + if (user_shstk) + set_cpu_cap(c, X86_FEATURE_USER_SHSTK); + + if (kernel_ibt) + msr = CET_ENDBR_EN; + wrmsrl(MSR_IA32_S_CET, msr); cr4_set_bits(X86_CR4_CET); - if (!ibt_selftest()) { + if (kernel_ibt && !ibt_selftest()) { pr_err("IBT selftest: Failed!\n"); wrmsrl(MSR_IA32_S_CET, 0); setup_clear_cpu_cap(X86_FEATURE_IBT); return; } } +#else /* CONFIG_X86_CET */ +static inline void setup_cet(struct cpuinfo_x86 *c) {} +#endif __noendbr void cet_disable(void) { - if (cpu_feature_enabled(X86_FEATURE_IBT)) - wrmsrl(MSR_IA32_S_CET, 0); + if (!(cpu_feature_enabled(X86_FEATURE_IBT) || + cpu_feature_enabled(X86_FEATURE_SHSTK))) + return; + + wrmsrl(MSR_IA32_S_CET, 0); + wrmsrl(MSR_IA32_U_CET, 0); } /* @@ -1470,6 +1492,9 @@ static void __init cpu_parse_early_param(void) if (cmdline_find_option_bool(boot_command_line, "noxsaves")) setup_clear_cpu_cap(X86_FEATURE_XSAVES); + if (cmdline_find_option_bool(boot_command_line, "nousershstk")) + setup_clear_cpu_cap(X86_FEATURE_USER_SHSTK); + arglen = cmdline_find_option(boot_command_line, "clearcpuid", arg, sizeof(arg)); if (arglen <= 0) return;