Hi Rick, On Fri, Dec 02, 2022 at 04:36:04PM -0800, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> > > Some applications (like GDB) would like to tweak shadow stack state via > ptrace. This allows for existing functionality to continue to work for > seized shadow stack applications. Provide an regset interface for > manipulating the shadow stack pointer (SSP). > > There is already ptrace functionality for accessing xstate, but this > does not include supervisor xfeatures. So there is not a completely > clear place for where to put the shadow stack state. Adding it to the > user xfeatures regset would complicate that code, as it currently shares > logic with signals which should not have supervisor features. > > Don't add a general supervisor xfeature regset like the user one, > because it is better to maintain flexibility for other supervisor > xfeatures to define their own interface. For example, an xfeature may > decide not to expose all of it's state to userspace, as is actually the > case for shadow stack ptrace functionality. A lot of enum values remain > to be used, so just put it in dedicated shadow stack regset. > > The only downside to not having a generic supervisor xfeature regset, > is that apps need to be enlightened of any new supervisor xfeature > exposed this way (i.e. they can't try to have generic save/restore > logic). But maybe that is a good thing, because they have to think > through each new xfeature instead of encountering issues when new a new > supervisor xfeature was added. > > By adding a shadow stack regset, it also has the effect of including the > shadow stack state in a core dump, which could be useful for debugging. > > The shadow stack specific xstate includes the SSP, and the shadow stack > and WRSS enablement status. Enabling shadow stack or wrss in the kernel > involves more than just flipping the bit. The kernel is made aware that > it has to do extra things when cloning or handling signals. That logic > is triggered off of separate feature enablement state kept in the task > struct. So the flipping on HW shadow stack enforcement without notifying > the kernel to change its behavior would severely limit what an application > could do without crashing, and the results would depend on kernel > internal implementation details. There is also no known use for controlling > this state via prtace today. So only expose the SSP, which is something > that userspace already has indirect control over. > > Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx> > Tested-by: John Allen <john.allen@xxxxxxx> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> > --- > > v4: > - Make shadow stack only. Reduce to only supporting SSP register, and > remove CET references (peterz) > - Add comment to not use 0x203, becuase binutils already looks for it in > coredumps. (Christina Schimpe) > > v3: > - Drop dependence on thread.shstk.size, and use thread.features bits > - Drop 32 bit support > > v2: > - Check alignment on ssp. > - Block IBT bits. > - Handle init states instead of returning error. > - Add verbose commit log justifying the design. > > Yu-Cheng v12: > - Return -ENODEV when CET registers are in INIT state. > - Check reserved/non-support bits from user input. > > arch/x86/include/asm/fpu/regset.h | 7 +-- > arch/x86/kernel/fpu/regset.c | 87 +++++++++++++++++++++++++++++++ > arch/x86/kernel/ptrace.c | 12 +++++ > include/uapi/linux/elf.h | 2 + > 4 files changed, 105 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/include/asm/fpu/regset.h b/arch/x86/include/asm/fpu/regset.h > index 4f928d6a367b..697b77e96025 100644 > --- a/arch/x86/include/asm/fpu/regset.h > +++ b/arch/x86/include/asm/fpu/regset.h > @@ -7,11 +7,12 @@ > > #include <linux/regset.h> > > -extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active; > +extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active, > + ssp_active; > extern user_regset_get2_fn fpregs_get, xfpregs_get, fpregs_soft_get, > - xstateregs_get; > + xstateregs_get, ssp_get; > extern user_regset_set_fn fpregs_set, xfpregs_set, fpregs_soft_set, > - xstateregs_set; > + xstateregs_set, ssp_set; > > /* > * xstateregs_active == regset_fpregs_active. Please refer to the comment > diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c > index 6d056b68f4ed..00f3d5c9b682 100644 > --- a/arch/x86/kernel/fpu/regset.c > +++ b/arch/x86/kernel/fpu/regset.c > @@ -8,6 +8,7 @@ > #include <asm/fpu/api.h> > #include <asm/fpu/signal.h> > #include <asm/fpu/regset.h> > +#include <asm/prctl.h> > > #include "context.h" > #include "internal.h" > @@ -174,6 +175,92 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, > return ret; > } > > + > +#ifdef CONFIG_X86_USER_SHADOW_STACK > +int ssp_active(struct task_struct *target, const struct user_regset *regset) > +{ > + if (shstk_enabled()) This is not going to work with ptrace as shstk_enabled() checks current rather than target. > + return regset->n; > + > + return 0; > +} > + > +int ssp_get(struct task_struct *target, const struct user_regset *regset, > + struct membuf to) > +{ > + struct fpu *fpu = &target->thread.fpu; > + struct cet_user_state *cetregs; > + > + if (!boot_cpu_has(X86_FEATURE_USER_SHSTK)) > + return -ENODEV; > + > + sync_fpstate(fpu); > + cetregs = get_xsave_addr(&fpu->fpstate->regs.xsave, XFEATURE_CET_USER); > + if (!cetregs) { > + /* > + * The registers are the in the init state. The init values for > + * these regs are zero, so just zero the output buffer. > + */ > + membuf_zero(&to, sizeof(cetregs->user_ssp)); > + return 0; > + } > + > + return membuf_write(&to, (unsigned long *)&cetregs->user_ssp, > + sizeof(cetregs->user_ssp)); > +} > + > +int ssp_set(struct task_struct *target, const struct user_regset *regset, > + unsigned int pos, unsigned int count, > + const void *kbuf, const void __user *ubuf) > +{ > + struct fpu *fpu = &target->thread.fpu; > + struct xregs_state *xsave = &fpu->fpstate->regs.xsave; > + struct cet_user_state *cetregs; > + unsigned long user_ssp; > + int r; > + > + if (!boot_cpu_has(X86_FEATURE_USER_SHSTK) || > + !ssp_active(target, regset)) > + return -ENODEV; > + > + r = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &user_ssp, 0, -1); > + if (r) > + return r; > + > + /* > + * Some kernel instructions (IRET, etc) can cause exceptions in the case > + * of disallowed CET register values. Just prevent invalid values. > + */ > + if ((user_ssp >= TASK_SIZE_MAX) || !IS_ALIGNED(user_ssp, 8)) > + return -EINVAL; > + > + fpu_force_restore(fpu); > + > + /* > + * Don't want to init the xfeature until the kernel will definetely > + * overwrite it, otherwise if it inits and then fails out, it would > + * end up initing it to random data. > + */ > + if (!xfeature_saved(xsave, XFEATURE_CET_USER) && > + WARN_ON(init_xfeature(xsave, XFEATURE_CET_USER))) > + return -ENODEV; > + > + cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER); > + if (WARN_ON(!cetregs)) { > + /* > + * This shouldn't ever be NULL because it was successfully > + * inited above if needed. The only scenario would be if an > + * xfeature was somehow saved in a buffer, but not enabled in > + * xsave. > + */ > + return -ENODEV; > + } > + > + cetregs->user_ssp = user_ssp; > + return 0; > +} > +#endif /* CONFIG_X86_USER_SHADOW_STACK */ > + > #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION > > /* > diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c > index dfaa270a7cc9..095f04bdabdc 100644 > --- a/arch/x86/kernel/ptrace.c > +++ b/arch/x86/kernel/ptrace.c > @@ -58,6 +58,7 @@ enum x86_regset_64 { > REGSET64_FP, > REGSET64_IOPERM, > REGSET64_XSTATE, > + REGSET64_SSP, > }; > > #define REGSET_GENERAL \ > @@ -1267,6 +1268,17 @@ static struct user_regset x86_64_regsets[] __ro_after_init = { > .active = ioperm_active, > .regset_get = ioperm_get > }, > +#ifdef CONFIG_X86_USER_SHADOW_STACK > + [REGSET64_SSP] = { > + .core_note_type = NT_X86_SHSTK, > + .n = 1, > + .size = sizeof(u64), > + .align = sizeof(u64), > + .active = ssp_active, > + .regset_get = ssp_get, > + .set = ssp_set > + }, > +#endif > }; > > static const struct user_regset_view user_x86_64_view = { > diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h > index c7b056af9ef0..e9283f0641c4 100644 > --- a/include/uapi/linux/elf.h > +++ b/include/uapi/linux/elf.h > @@ -406,6 +406,8 @@ typedef struct elf64_shdr { > #define NT_386_TLS 0x200 /* i386 TLS slots (struct user_desc) */ > #define NT_386_IOPERM 0x201 /* x86 io permission bitmap (1=deny) */ > #define NT_X86_XSTATE 0x202 /* x86 extended state using xsave */ > +/* Old binutils treats 0x203 as a CET state */ > +#define NT_X86_SHSTK 0x204 /* x86 SHSTK state */ > #define NT_S390_HIGH_GPRS 0x300 /* s390 upper register halves */ > #define NT_S390_TIMER 0x301 /* s390 timer register */ > #define NT_S390_TODCMP 0x302 /* s390 TOD clock comparator register */ > -- > 2.17.1 > -- Sincerely yours, Mike.