| Message ID | 20210127212524.10188-6-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control-flow Enforcement: Shadow Stack | expand |
On 1/27/21 1:25 PM, Yu-cheng Yu wrote: > @@ -135,6 +135,8 @@ enum xfeature { > #define XFEATURE_MASK_PT (1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR) > #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU) > #define XFEATURE_MASK_PASID (1 << XFEATURE_PASID) > +#define XFEATURE_MASK_CET_USER (1 << XFEATURE_CET_USER) > +#define XFEATURE_MASK_CET_KERNEL (1 << XFEATURE_CET_KERNEL) > #define XFEATURE_MASK_LBR (1 << XFEATURE_LBR) > > #define XFEATURE_MASK_FPSSE (XFEATURE_MASK_FP | XFEATURE_MASK_SSE) > @@ -237,6 +239,23 @@ struct pkru_state { > u32 pad; > } __packed; > > +/* > + * State component 11 is Control-flow Enforcement user states > + */ > +struct cet_user_state { > + u64 user_cet; /* user control-flow settings */ > + u64 user_ssp; /* user shadow stack pointer */ > +}; Andy Cooper just mentioned on IRC about this nugget in the spec: XRSTORS on CET state will do reserved bit and canonicality checks on the state in similar manner as done by the WRMSR to these state elements. We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. Could we prove this out in practice, please?
On 1/29/2021 1:00 PM, Dave Hansen wrote: > On 1/27/21 1:25 PM, Yu-cheng Yu wrote: >> @@ -135,6 +135,8 @@ enum xfeature { >> #define XFEATURE_MASK_PT (1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR) >> #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU) >> #define XFEATURE_MASK_PASID (1 << XFEATURE_PASID) >> +#define XFEATURE_MASK_CET_USER (1 << XFEATURE_CET_USER) >> +#define XFEATURE_MASK_CET_KERNEL (1 << XFEATURE_CET_KERNEL) >> #define XFEATURE_MASK_LBR (1 << XFEATURE_LBR) >> >> #define XFEATURE_MASK_FPSSE (XFEATURE_MASK_FP | XFEATURE_MASK_SSE) >> @@ -237,6 +239,23 @@ struct pkru_state { >> u32 pad; >> } __packed; >> >> +/* >> + * State component 11 is Control-flow Enforcement user states >> + */ >> +struct cet_user_state { >> + u64 user_cet; /* user control-flow settings */ >> + u64 user_ssp; /* user shadow stack pointer */ >> +}; > > Andy Cooper just mentioned on IRC about this nugget in the spec: > > XRSTORS on CET state will do reserved bit and canonicality > checks on the state in similar manner as done by the WRMSR to > these state elements. > > We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. > Could we prove this out in practice, please? > Do we want to verify that setting reserved bits in CET XSAVES states triggers GP? Then, yes, I just verified it again. Thanks for reminding. Do we have any particular case relating to this? -- Yu-cheng
On 1/29/21 2:35 PM, Yu, Yu-cheng wrote: >> Andy Cooper just mentioned on IRC about this nugget in the spec: >> >> XRSTORS on CET state will do reserved bit and canonicality >> checks on the state in similar manner as done by the WRMSR to >> these state elements. >> >> We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. >> Could we prove this out in practice, please? >>> > Do we want to verify that setting reserved bits in CET XSAVES states > triggers GP? Then, yes, I just verified it again. Thanks for > reminding. Do we have any particular case relating to this? I want to confirm that it triggers #GP and kills userspace without the kernel WARN'ing or otherwise being visibly unhappy. What about the return-to-userspace path after a ptracer writes content to the CET fields? I don't see the same tolerance for errors in __fpregs_load_activate(), for instance.
On 1/29/2021 2:53 PM, Dave Hansen wrote: > On 1/29/21 2:35 PM, Yu, Yu-cheng wrote: >>> Andy Cooper just mentioned on IRC about this nugget in the spec: >>> >>> XRSTORS on CET state will do reserved bit and canonicality >>> checks on the state in similar manner as done by the WRMSR to >>> these state elements. >>> >>> We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. >>> Could we prove this out in practice, please? >>>> >> Do we want to verify that setting reserved bits in CET XSAVES states >> triggers GP? Then, yes, I just verified it again. Thanks for >> reminding. Do we have any particular case relating to this? > > I want to confirm that it triggers #GP and kills userspace without the > kernel WARN'ing or otherwise being visibly unhappy. For sigreturn, shadow stack pointer is checked against its restore token and must be smaller than TASK_SIZE_MAX. Sigreturn cannot set any MSR_IA32_U_CET reserved bits. > > What about the return-to-userspace path after a ptracer writes content > to the CET fields? I don't see the same tolerance for errors in > __fpregs_load_activate(), for instance. > Good thought. I have not sent out my revised PTRACE patch, but values from user will be checked for valid address and reserved bits. -- Yu-cheng
On 2/1/21 2:43 PM, Yu, Yu-cheng wrote: > On 1/29/2021 2:53 PM, Dave Hansen wrote: >> On 1/29/21 2:35 PM, Yu, Yu-cheng wrote: >>>> Andy Cooper just mentioned on IRC about this nugget in the spec: >>>> >>>> XRSTORS on CET state will do reserved bit and canonicality >>>> checks on the state in similar manner as done by the WRMSR to >>>> these state elements. >>>> >>>> We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. >>>> Could we prove this out in practice, please? >>>>> >>> Do we want to verify that setting reserved bits in CET XSAVES states >>> triggers GP? Then, yes, I just verified it again. Thanks for >>> reminding. Do we have any particular case relating to this? >> >> I want to confirm that it triggers #GP and kills userspace without the >> kernel WARN'ing or otherwise being visibly unhappy. > > For sigreturn, shadow stack pointer is checked against its restore token > and must be smaller than TASK_SIZE_MAX. Sigreturn cannot set any > MSR_IA32_U_CET reserved bits. That would be nice to at least allude to in the changelog or comments. >> What about the return-to-userspace path after a ptracer writes content >> to the CET fields? I don't see the same tolerance for errors in >> __fpregs_load_activate(), for instance. >> > > Good thought. I have not sent out my revised PTRACE patch, but values > from user will be checked for valid address and reserved bits. Wait a sec... What about *THIS* series? Will *THIS* series give us oopses when userspace blasts a new XSAVE buffer in with NT_X86_XSTATE?
On 2/1/2021 2:59 PM, Dave Hansen wrote: > On 2/1/21 2:43 PM, Yu, Yu-cheng wrote: >> On 1/29/2021 2:53 PM, Dave Hansen wrote: >>> On 1/29/21 2:35 PM, Yu, Yu-cheng wrote: >>>>> Andy Cooper just mentioned on IRC about this nugget in the spec: >>>>> >>>>> XRSTORS on CET state will do reserved bit and canonicality >>>>> checks on the state in similar manner as done by the WRMSR to >>>>> these state elements. >>>>> >>>>> We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. >>>>> Could we prove this out in practice, please? >>>>>> >>>> Do we want to verify that setting reserved bits in CET XSAVES states >>>> triggers GP? Then, yes, I just verified it again. Thanks for >>>> reminding. Do we have any particular case relating to this? >>> >>> I want to confirm that it triggers #GP and kills userspace without the >>> kernel WARN'ing or otherwise being visibly unhappy. >> >> For sigreturn, shadow stack pointer is checked against its restore token >> and must be smaller than TASK_SIZE_MAX. Sigreturn cannot set any >> MSR_IA32_U_CET reserved bits. > > That would be nice to at least allude to in the changelog or comments. > Ok. >>> What about the return-to-userspace path after a ptracer writes content >>> to the CET fields? I don't see the same tolerance for errors in >>> __fpregs_load_activate(), for instance. >>> >> >> Good thought. I have not sent out my revised PTRACE patch, but values >> from user will be checked for valid address and reserved bits. > > Wait a sec... What about *THIS* series? Will *THIS* series give us > oopses when userspace blasts a new XSAVE buffer in with NT_X86_XSTATE? > Fortunately, CET states are supervisor states. NT_x86_XSTATE has only user states.
On 2/1/21 3:05 PM, Yu, Yu-cheng wrote: >>> >> >> Wait a sec... What about *THIS* series? Will *THIS* series give us >> oopses when userspace blasts a new XSAVE buffer in with NT_X86_XSTATE? >> > > Fortunately, CET states are supervisor states. NT_x86_XSTATE has only > user states. Ahhh, good point. You did mention this in the changelog: > Control-flow Enforcement Technology (CET) introduces these MSRs: > > MSR_IA32_U_CET (user-mode CET settings), > MSR_IA32_PL3_SSP (user-mode shadow stack pointer), > > MSR_IA32_PL0_SSP (kernel-mode shadow stack pointer), > MSR_IA32_PL1_SSP (Privilege Level 1 shadow stack pointer), > MSR_IA32_PL2_SSP (Privilege Level 2 shadow stack pointer), > MSR_IA32_S_CET (kernel-mode CET settings), > MSR_IA32_INT_SSP_TAB (exception shadow stack table). > > The two user-mode MSRs belong to XFEATURE_CET_USER. The first three of > kernel-mode MSRs belong to XFEATURE_CET_KERNEL. Both XSAVES states are > supervisor states. This is another great place to add some information about the feature. "Both XSAVES states are supervisor states." ... This means that there is no direct, unprivileged access to this state, making it harder for an attacker to subvert CET. You could also allude to the future ptrace() support here.
On 2/1/2021 3:12 PM, Dave Hansen wrote: > On 2/1/21 3:05 PM, Yu, Yu-cheng wrote: >>>> >>> >>> Wait a sec... What about *THIS* series? Will *THIS* series give us >>> oopses when userspace blasts a new XSAVE buffer in with NT_X86_XSTATE? >>> >> >> Fortunately, CET states are supervisor states. NT_x86_XSTATE has only >> user states. > > Ahhh, good point. You did mention this in the changelog: > >> Control-flow Enforcement Technology (CET) introduces these MSRs: >> >> MSR_IA32_U_CET (user-mode CET settings), >> MSR_IA32_PL3_SSP (user-mode shadow stack pointer), >> >> MSR_IA32_PL0_SSP (kernel-mode shadow stack pointer), >> MSR_IA32_PL1_SSP (Privilege Level 1 shadow stack pointer), >> MSR_IA32_PL2_SSP (Privilege Level 2 shadow stack pointer), >> MSR_IA32_S_CET (kernel-mode CET settings), >> MSR_IA32_INT_SSP_TAB (exception shadow stack table). >> >> The two user-mode MSRs belong to XFEATURE_CET_USER. The first three of >> kernel-mode MSRs belong to XFEATURE_CET_KERNEL. Both XSAVES states are >> supervisor states. > > This is another great place to add some information about the feature. > > "Both XSAVES states are supervisor states." ... This means that there > is no direct, unprivileged access to this state, making it harder for an > attacker to subvert CET. > > You could also allude to the future ptrace() support here. > I will add that.
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h index f5a38a5f3ae1..035eb0ec665e 100644 --- a/arch/x86/include/asm/fpu/types.h +++ b/arch/x86/include/asm/fpu/types.h @@ -115,8 +115,8 @@ enum xfeature { XFEATURE_PT_UNIMPLEMENTED_SO_FAR, XFEATURE_PKRU, XFEATURE_PASID, - XFEATURE_RSRVD_COMP_11, - XFEATURE_RSRVD_COMP_12, + XFEATURE_CET_USER, + XFEATURE_CET_KERNEL, XFEATURE_RSRVD_COMP_13, XFEATURE_RSRVD_COMP_14, XFEATURE_LBR, @@ -135,6 +135,8 @@ enum xfeature { #define XFEATURE_MASK_PT (1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR) #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU) #define XFEATURE_MASK_PASID (1 << XFEATURE_PASID) +#define XFEATURE_MASK_CET_USER (1 << XFEATURE_CET_USER) +#define XFEATURE_MASK_CET_KERNEL (1 << XFEATURE_CET_KERNEL) #define XFEATURE_MASK_LBR (1 << XFEATURE_LBR) #define XFEATURE_MASK_FPSSE (XFEATURE_MASK_FP | XFEATURE_MASK_SSE) @@ -237,6 +239,23 @@ struct pkru_state { u32 pad; } __packed; +/* + * State component 11 is Control-flow Enforcement user states + */ +struct cet_user_state { + u64 user_cet; /* user control-flow settings */ + u64 user_ssp; /* user shadow stack pointer */ +}; + +/* + * State component 12 is Control-flow Enforcement kernel states + */ +struct cet_kernel_state { + u64 kernel_ssp; /* kernel shadow stack */ + u64 pl1_ssp; /* privilege level 1 shadow stack */ + u64 pl2_ssp; /* privilege level 2 shadow stack */ +}; + /* * State component 15: Architectural LBR configuration state. * The size of Arch LBR state depends on the number of LBRs (lbr_depth). diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index 47a92232d595..582f3575e0bd 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -35,7 +35,8 @@ XFEATURE_MASK_BNDCSR) /* All currently supported supervisor features */ -#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID) +#define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID | \ + XFEATURE_MASK_CET_USER) /* * A supervisor state component may not always contain valuable information, @@ -62,7 +63,8 @@ * Unsupported supervisor features. When a supervisor feature in this mask is * supported in the future, move it to the supported supervisor feature mask. */ -#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT) +#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT | \ + XFEATURE_MASK_CET_KERNEL) /* All supervisor states including supported and unsupported states. */ #define XFEATURE_MASK_SUPERVISOR_ALL (XFEATURE_MASK_SUPERVISOR_SUPPORTED | \ diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index 546d6ecf0a35..fae6b3ea1f6d 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -933,4 +933,23 @@ #define MSR_VM_IGNNE 0xc0010115 #define MSR_VM_HSAVE_PA 0xc0010117 +/* Control-flow Enforcement Technology MSRs */ +#define MSR_IA32_U_CET 0x6a0 /* user mode cet setting */ +#define MSR_IA32_S_CET 0x6a2 /* kernel mode cet setting */ +#define CET_SHSTK_EN BIT_ULL(0) +#define CET_WRSS_EN BIT_ULL(1) +#define CET_ENDBR_EN BIT_ULL(2) +#define CET_LEG_IW_EN BIT_ULL(3) +#define CET_NO_TRACK_EN BIT_ULL(4) +#define CET_SUPPRESS_DISABLE BIT_ULL(5) +#define CET_RESERVED (BIT_ULL(6) | BIT_ULL(7) | BIT_ULL(8) | BIT_ULL(9)) +#define CET_SUPPRESS BIT_ULL(10) +#define CET_WAIT_ENDBR BIT_ULL(11) + +#define MSR_IA32_PL0_SSP 0x6a4 /* kernel shadow stack pointer */ +#define MSR_IA32_PL1_SSP 0x6a5 /* ring-1 shadow stack pointer */ +#define MSR_IA32_PL2_SSP 0x6a6 /* ring-2 shadow stack pointer */ +#define MSR_IA32_PL3_SSP 0x6a7 /* user shadow stack pointer */ +#define MSR_IA32_INT_SSP_TAB 0x6a8 /* exception shadow stack table */ + #endif /* _ASM_X86_MSR_INDEX_H */ diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 5d8047441a0a..22eedf8066bf 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -38,6 +38,8 @@ static const char *xfeature_names[] = "Processor Trace (unused)" , "Protection Keys User registers", "PASID state", + "Control-flow User registers" , + "Control-flow Kernel registers" , "unknown xstate feature" , }; @@ -53,6 +55,8 @@ static short xsave_cpuid_features[] __initdata = { X86_FEATURE_INTEL_PT, X86_FEATURE_PKU, X86_FEATURE_ENQCMD, + X86_FEATURE_CET, /* XFEATURE_CET_USER */ + X86_FEATURE_CET, /* XFEATURE_CET_KERNEL */ }; /* @@ -321,6 +325,8 @@ static void __init print_xstate_features(void) print_xstate_feature(XFEATURE_MASK_Hi16_ZMM); print_xstate_feature(XFEATURE_MASK_PKRU); print_xstate_feature(XFEATURE_MASK_PASID); + print_xstate_feature(XFEATURE_MASK_CET_USER); + print_xstate_feature(XFEATURE_MASK_CET_KERNEL); } /* @@ -596,6 +602,8 @@ static void check_xstate_against_struct(int nr) XCHECK_SZ(sz, nr, XFEATURE_Hi16_ZMM, struct avx_512_hi16_state); XCHECK_SZ(sz, nr, XFEATURE_PKRU, struct pkru_state); XCHECK_SZ(sz, nr, XFEATURE_PASID, struct ia32_pasid_state); + XCHECK_SZ(sz, nr, XFEATURE_CET_USER, struct cet_user_state); + XCHECK_SZ(sz, nr, XFEATURE_CET_KERNEL, struct cet_kernel_state); /* * Make *SURE* to add any feature numbers in below if @@ -605,7 +613,7 @@ static void check_xstate_against_struct(int nr) if ((nr < XFEATURE_YMM) || (nr >= XFEATURE_MAX) || (nr == XFEATURE_PT_UNIMPLEMENTED_SO_FAR) || - ((nr >= XFEATURE_RSRVD_COMP_11) && (nr <= XFEATURE_LBR))) { + ((nr >= XFEATURE_RSRVD_COMP_13) && (nr <= XFEATURE_LBR))) { WARN_ONCE(1, "no structure for xstate: %d\n", nr); XSTATE_WARN_ON(1); }
Control-flow Enforcement Technology (CET) introduces these MSRs: MSR_IA32_U_CET (user-mode CET settings), MSR_IA32_PL3_SSP (user-mode shadow stack pointer), MSR_IA32_PL0_SSP (kernel-mode shadow stack pointer), MSR_IA32_PL1_SSP (Privilege Level 1 shadow stack pointer), MSR_IA32_PL2_SSP (Privilege Level 2 shadow stack pointer), MSR_IA32_S_CET (kernel-mode CET settings), MSR_IA32_INT_SSP_TAB (exception shadow stack table). The two user-mode MSRs belong to XFEATURE_CET_USER. The first three of kernel-mode MSRs belong to XFEATURE_CET_KERNEL. Both XSAVES states are supervisor states. Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> --- arch/x86/include/asm/fpu/types.h | 23 +++++++++++++++++++++-- arch/x86/include/asm/fpu/xstate.h | 6 ++++-- arch/x86/include/asm/msr-index.h | 19 +++++++++++++++++++ arch/x86/kernel/fpu/xstate.c | 10 +++++++++- 4 files changed, 53 insertions(+), 5 deletions(-)