On Mon, 2022-10-03 at 13:44 -0700, Kees Cook wrote: > On Thu, Sep 29, 2022 at 03:29:23PM -0700, Rick Edgecombe wrote: > > From: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> > > > > Shadow stack's are normally written to via CALL/RET or specific CET > > instuctions like RSTORSSP/SAVEPREVSSP. However during some Linux > > operations the kernel will need to write to directly using the > > ring-0 only > > WRUSS instruction. > > > > A shadow stack restore token marks a restore point of the shadow > > stack, and > > the address in a token must point directly above the token, which > > is within > > the same shadow stack. This is distinctively different from other > > pointers > > on the shadow stack, since those pointers point to executable code > > area. > > > > Introduce token setup and verify routines. Also introduce WRUSS, > > which is > > a kernel-mode instruction but writes directly to user shadow stack. > > > > In future patches that enable shadow stack to work with signals, > > the kernel > > will need something to denote the point in the stack where > > sigreturn may be > > called. This will prevent attackers calling sigreturn at arbitrary > > places > > in the stack, in order to help prevent SROP attacks. > > > > To do this, something that can only be written by the kernel needs > > to be > > placed on the shadow stack. This can be accomplished by setting bit > > 63 in > > the frame written to the shadow stack. Userspace return addresses > > can't > > have this bit set as it is in the kernel range. It is also can't be > > a > > valid restore token. > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> > > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > > > > --- > > > > v2: > > - Add data helpers for writing to shadow stack. > > > > v1: > > - Use xsave helpers. > > > > Yu-cheng v30: > > - Update commit log, remove description about signals. > > - Update various comments. > > - Remove variable 'ssp' init and adjust return value accordingly. > > - Check get_user_shstk_addr() return value. > > - Replace 'ia32' with 'proc32'. > > > > Yu-cheng v29: > > - Update comments for the use of get_xsave_addr(). > > > > arch/x86/include/asm/special_insns.h | 13 ++++ > > arch/x86/kernel/shstk.c | 108 > > +++++++++++++++++++++++++++ > > 2 files changed, 121 insertions(+) > > > > diff --git a/arch/x86/include/asm/special_insns.h > > b/arch/x86/include/asm/special_insns.h > > index 35f709f619fb..f096f52bd059 100644 > > --- a/arch/x86/include/asm/special_insns.h > > +++ b/arch/x86/include/asm/special_insns.h > > @@ -223,6 +223,19 @@ static inline void clwb(volatile void *__p) > > : [pax] "a" (p)); > > } > > > > +#ifdef CONFIG_X86_SHADOW_STACK > > +static inline int write_user_shstk_64(u64 __user *addr, u64 val) > > +{ > > + asm_volatile_goto("1: wrussq %[val], (%[addr])\n" > > + _ASM_EXTABLE(1b, %l[fail]) > > + :: [addr] "r" (addr), [val] "r" (val) > > + :: fail); > > + return 0; > > +fail: > > + return -EFAULT; > > +} > > +#endif /* CONFIG_X86_SHADOW_STACK */ > > + > > #define nop() asm volatile ("nop") > > > > static inline void serialize(void) > > diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c > > index db4e53f9fdaf..8904aef487bf 100644 > > --- a/arch/x86/kernel/shstk.c > > +++ b/arch/x86/kernel/shstk.c > > @@ -25,6 +25,8 @@ > > #include <asm/fpu/api.h> > > #include <asm/prctl.h> > > > > +#define SS_FRAME_SIZE 8 > > + > > static bool feature_enabled(unsigned long features) > > { > > return current->thread.features & features; > > @@ -40,6 +42,31 @@ static void feature_clr(unsigned long features) > > current->thread.features &= ~features; > > } > > > > +/* > > + * Create a restore token on the shadow stack. A token is always > > 8-byte > > + * and aligned to 8. > > + */ > > +static int create_rstor_token(unsigned long ssp, unsigned long > > *token_addr) > > +{ > > + unsigned long addr; > > + > > + /* Token must be aligned */ > > + if (!IS_ALIGNED(ssp, 8)) > > + return -EINVAL; > > + > > + addr = ssp - SS_FRAME_SIZE; > > + > > + /* Mark the token 64-bit */ > > + ssp |= BIT(0); > > Wow, that confused me for a moment. :) SDE says: > > - Bit 63:2 – Value of shadow stack pointer when this restore point > was created. > - Bit 1 – Reserved. Must be zero. > - Bit 0 – Mode bit. If 0, the token is a compatibility/legacy mode > “shadow stack restore” token. If 1, then this shadow stack > restore > token can be used with a RSTORSSP instruction in 64-bit > mode. > > So shouldn't this actually be: > > ssp &= ~BIT(1); /* Reserved */ > ssp |= BIT(0); /* RSTORSSP instruction in 64-bit mode */ Since SSP is aligned, it should not have bits 0 to 2 set. I'll add a comment. > > > + > > + if (write_user_shstk_64((u64 __user *)addr, (u64)ssp)) > > + return -EFAULT; > > + > > + *token_addr = addr; > > + > > + return 0; > > +} > > + > > static unsigned long alloc_shstk(unsigned long size) > > { > > int flags = MAP_ANONYMOUS | MAP_PRIVATE; > > @@ -158,6 +185,87 @@ int shstk_alloc_thread_stack(struct > > task_struct *tsk, unsigned long clone_flags, > > return 0; > > } > > > > +static unsigned long get_user_shstk_addr(void) > > +{ > > + unsigned long long ssp; > > + > > + fpu_lock_and_load(); > > + > > + rdmsrl(MSR_IA32_PL3_SSP, ssp); > > + > > + fpregs_unlock(); > > + > > + return ssp; > > +} > > + > > +static int put_shstk_data(u64 __user *addr, u64 data) > > +{ > > + WARN_ON(data & BIT(63)); > > Let's make this a bit more defensive: > > if (WARN_ON_ONCE(data & BIT(63))) > return -EFAULT; Hmm, sure. I'm thinking EINVAL since the failure has nothing to do with faulting. > > > + > > + /* > > + * Mark the high bit so that the sigframe can't be processed as > > a > > + * return address. > > + */ > > + if (write_user_shstk_64(addr, data | BIT(63))) > > + return -EFAULT; > > + return 0; > > +} > > + > > +static int get_shstk_data(unsigned long *data, unsigned long > > __user *addr) > > +{ > > + unsigned long ldata; > > + > > + if (unlikely(get_user(ldata, addr))) > > + return -EFAULT; > > + > > + if (!(ldata & BIT(63))) > > + return -EINVAL; > > + > > + *data = ldata & ~BIT(63); > > + > > + return 0; > > +} > > + > > +/* > > + * Verify the user shadow stack has a valid token on it, and then > > set > > + * *new_ssp according to the token. > > + */ > > +static int shstk_check_rstor_token(unsigned long *new_ssp) > > +{ > > + unsigned long token_addr; > > + unsigned long token; > > + > > + token_addr = get_user_shstk_addr(); > > + if (!token_addr) > > + return -EINVAL; > > + > > + if (get_user(token, (unsigned long __user *)token_addr)) > > + return -EFAULT; > > + > > + /* Is mode flag correct? */ > > + if (!(token & BIT(0))) > > + return -EINVAL; > > + > > + /* Is busy flag set? */ > > "Busy"? Not "Reserved"? Yes reserved is more accurate, I'll change it. In a previous-ssp token it is 1, so kind of busy-like. That is probably where the comment came from. > > > + if (token & BIT(1)) > > + return -EINVAL; > > + > > + /* Mask out flags */ > > + token &= ~3UL; > > + > > + /* Restore address aligned? */ > > + if (!IS_ALIGNED(token, 8)) > > + return -EINVAL; > > + > > + /* Token placed properly? */ > > + if (((ALIGN_DOWN(token, 8) - 8) != token_addr) || token >= > > TASK_SIZE_MAX) > > + return -EINVAL; > > + > > + *new_ssp = token; > > + > > + return 0; > > +} > > + > > void shstk_free(struct task_struct *tsk) > > { > > struct thread_shstk *shstk = &tsk->thread.shstk; > > -- > > 2.17.1 > > > >