| Message ID | 20220130211838.8382-35-rick.p.edgecombe@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Shadow stacks for userspace | expand |
* Rick Edgecombe: > For the current shadow stack implementation, shadow stacks contents cannot > be arbitrarily provisioned with data. This property helps apps protect > themselves better, but also restricts any potential apps that may want to > do exotic things at the expense of a little security. > > The x86 shadow stack feature introduces a new instruction, wrss, which > can be enabled to write directly to shadow stack permissioned memory from > userspace. Allow it to get enabled via the prctl interface. Why can't this be turned on unconditionally? Thanks, Florian
On Sun, Jan 30, 2022 at 11:57 PM Florian Weimer <fweimer@redhat.com> wrote: > > * Rick Edgecombe: > > > For the current shadow stack implementation, shadow stacks contents cannot > > be arbitrarily provisioned with data. This property helps apps protect > > themselves better, but also restricts any potential apps that may want to > > do exotic things at the expense of a little security. > > > > The x86 shadow stack feature introduces a new instruction, wrss, which > > can be enabled to write directly to shadow stack permissioned memory from > > userspace. Allow it to get enabled via the prctl interface. > > Why can't this be turned on unconditionally? WRSS can be a security risk since it defeats the whole purpose of Shadow Stack. If an application needs to write to shadow stack, it can make a syscall to enable it. After the CET patches are checked in Linux kernel, I will make a proposal to allow applications or shared libraries to opt-in WRSS through a linker option, a compiler option or a function attribute.
* H. J. Lu: > On Sun, Jan 30, 2022 at 11:57 PM Florian Weimer <fweimer@redhat.com> wrote: >> >> * Rick Edgecombe: >> >> > For the current shadow stack implementation, shadow stacks contents cannot >> > be arbitrarily provisioned with data. This property helps apps protect >> > themselves better, but also restricts any potential apps that may want to >> > do exotic things at the expense of a little security. >> > >> > The x86 shadow stack feature introduces a new instruction, wrss, which >> > can be enabled to write directly to shadow stack permissioned memory from >> > userspace. Allow it to get enabled via the prctl interface. >> >> Why can't this be turned on unconditionally? > > WRSS can be a security risk since it defeats the whole purpose of > Shadow Stack. If an application needs to write to shadow stack, > it can make a syscall to enable it. After the CET patches are checked > in Linux kernel, I will make a proposal to allow applications or shared > libraries to opt-in WRSS through a linker option, a compiler option or > a function attribute. Ahh, that makes sense. I assumed that without WRSS, the default was to allow plain writes. 8-) Thanks, Florian
diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h index cbc7cfcba5dc..c8ff0bd5f5bc 100644 --- a/arch/x86/include/asm/cet.h +++ b/arch/x86/include/asm/cet.h @@ -10,6 +10,7 @@ struct task_struct; struct thread_shstk { u64 base; u64 size; + bool wrss; }; #ifdef CONFIG_X86_SHADOW_STACK @@ -19,6 +20,7 @@ int shstk_alloc_thread_stack(struct task_struct *p, unsigned long clone_flags, void shstk_free(struct task_struct *p); int shstk_disable(void); void reset_thread_shstk(void); +int wrss_control(bool enable); int shstk_setup_rstor_token(bool proc32, unsigned long restorer, unsigned long *new_ssp); int shstk_check_rstor_token(bool proc32, unsigned long *new_ssp); @@ -32,6 +34,7 @@ static inline int shstk_alloc_thread_stack(struct task_struct *p, static inline void shstk_free(struct task_struct *p) {} static inline void shstk_disable(void) {} static inline void reset_thread_shstk(void) {} +static inline void wrss_control(bool enable) {} static inline int shstk_setup_rstor_token(bool proc32, unsigned long restorer, unsigned long *new_ssp) { return 0; } static inline int shstk_check_rstor_token(bool proc32, diff --git a/arch/x86/include/asm/mman.h b/arch/x86/include/asm/mman.h index b44fe31deb3a..c05951a36d93 100644 --- a/arch/x86/include/asm/mman.h +++ b/arch/x86/include/asm/mman.h @@ -8,7 +8,10 @@ #ifdef CONFIG_X86_SHADOW_STACK static inline bool arch_validate_flags(unsigned long vm_flags) { - if ((vm_flags & VM_SHADOW_STACK) && (vm_flags & VM_WRITE)) + /* + * Shadow stack must not be executable, to help with W^X due to wrss. + */ + if ((vm_flags & VM_SHADOW_STACK) && (vm_flags & (VM_WRITE | VM_EXEC))) return false; return true; diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h index aa294c7bcf41..210976925325 100644 --- a/arch/x86/include/uapi/asm/prctl.h +++ b/arch/x86/include/uapi/asm/prctl.h @@ -28,6 +28,6 @@ /* x86 feature bits to be used with ARCH_X86_FEATURE arch_prctl()s */ #define LINUX_X86_FEATURE_IBT 0x00000001 #define LINUX_X86_FEATURE_SHSTK 0x00000002 - +#define LINUX_X86_FEATURE_WRSS 0x00000010 #endif /* _ASM_X86_PRCTL_H */ diff --git a/arch/x86/kernel/elf_feature_prctl.c b/arch/x86/kernel/elf_feature_prctl.c index 47de201db3f7..ecad6ebeb4dd 100644 --- a/arch/x86/kernel/elf_feature_prctl.c +++ b/arch/x86/kernel/elf_feature_prctl.c @@ -21,6 +21,8 @@ static int elf_feat_copy_status_to_user(struct thread_shstk *shstk, u64 __user * buf[1] = shstk->base; buf[2] = shstk->size; } + if (shstk->wrss) + buf[0] |= LINUX_X86_FEATURE_WRSS; return copy_to_user(ubuf, buf, sizeof(buf)); } @@ -40,6 +42,8 @@ int prctl_elf_feature(int option, u64 arg2) if (arg2 & thread->feat_prctl_locked) return -EPERM; + if (arg2 & LINUX_X86_FEATURE_WRSS && !wrss_control(false)) + feat_succ |= LINUX_X86_FEATURE_WRSS; if (arg2 & LINUX_X86_FEATURE_SHSTK && !shstk_disable()) feat_succ |= LINUX_X86_FEATURE_SHSTK; @@ -52,6 +56,8 @@ int prctl_elf_feature(int option, u64 arg2) if (arg2 & LINUX_X86_FEATURE_SHSTK && !shstk_setup()) feat_succ |= LINUX_X86_FEATURE_SHSTK; + if (arg2 & LINUX_X86_FEATURE_WRSS && !wrss_control(true)) + feat_succ |= LINUX_X86_FEATURE_WRSS; if (feat_succ != arg2) return -ECANCELED; diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c index 53be5d5539d4..92612236b4ef 100644 --- a/arch/x86/kernel/shstk.c +++ b/arch/x86/kernel/shstk.c @@ -230,6 +230,36 @@ void shstk_free(struct task_struct *tsk) shstk->size = 0; } +int wrss_control(bool enable) +{ + struct thread_shstk *shstk = ¤t->thread.shstk; + void *xstate; + int err; + + if (!cpu_feature_enabled(X86_FEATURE_SHSTK)) + return 1; + /* + * Only enable wrss if shadow stack is enabled. If shadow stack is not + * enabled, wrss will already be disabled, so don't bother clearing it + * when disabling. + */ + if (!shstk->size || shstk->wrss == enable) + return 1; + + xstate = start_update_xsave_msrs(XFEATURE_CET_USER); + if (enable) + err = xsave_set_clear_bits_msrl(xstate, MSR_IA32_U_CET, CET_WRSS_EN, 0); + else + err = xsave_set_clear_bits_msrl(xstate, MSR_IA32_U_CET, 0, CET_WRSS_EN); + end_update_xsave_msrs(); + + if (err) + return 1; + + shstk->wrss = enable; + return 0; +} + int shstk_disable(void) { struct thread_shstk *shstk = ¤t->thread.shstk; @@ -242,7 +272,9 @@ int shstk_disable(void) return 1; xstate = start_update_xsave_msrs(XFEATURE_CET_USER); - err = xsave_set_clear_bits_msrl(xstate, MSR_IA32_U_CET, 0, CET_SHSTK_EN); + /* Disable WRSS too when disabling shadow stack */ + err = xsave_set_clear_bits_msrl(xstate, MSR_IA32_U_CET, 0, + CET_SHSTK_EN | CET_WRSS_EN); if (!err) err = xsave_wrmsrl(xstate, MSR_IA32_PL3_SSP, 0); end_update_xsave_msrs(); @@ -251,6 +283,7 @@ int shstk_disable(void) return 1; shstk_free(current); + shstk->wrss = 0; return 0; }
For the current shadow stack implementation, shadow stacks contents cannot be arbitrarily provisioned with data. This property helps apps protect themselves better, but also restricts any potential apps that may want to do exotic things at the expense of a little security. The x86 shadow stack feature introduces a new instruction, wrss, which can be enabled to write directly to shadow stack permissioned memory from userspace. Allow it to get enabled via the prctl interface. Only enable the userspace wrss instruction, which allows writes to userspace shadow stacks from userspace. Do not allow it to be enabled independently of shadow stack, as HW does not support using WRSS when shadow stack is disabled. Prevent shadow stack's from becoming executable to assist apps who want W^X enforced. Add an arch_validate_flags() implementation to handle the check. Rename the uapi/asm/mman.h header guard to be able to use it for arch/x86/include/asm/mman.h where the arch_validate_flags() will be. From a fault handler perspective, WRSS will behave very similar to WRUSS, which is treated like a user access from a PF err code perspective. Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> --- v1: - New patch. arch/x86/include/asm/cet.h | 3 +++ arch/x86/include/asm/mman.h | 5 ++++- arch/x86/include/uapi/asm/prctl.h | 2 +- arch/x86/kernel/elf_feature_prctl.c | 6 +++++ arch/x86/kernel/shstk.c | 35 ++++++++++++++++++++++++++++- 5 files changed, 48 insertions(+), 3 deletions(-)