Inter-revision diff: patch 34

Comparing v6 (message) to v8 (message)

--- v6
+++ v8
@@ -4,8 +4,8 @@
 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.
+can be enabled to write directly to shadow stack 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
@@ -15,14 +15,20 @@
 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>
+Reviewed-by: Kees Cook <keescook@chromium.org>
+Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>
 Tested-by: Pengfei Xu <pengfei.xu@intel.com>
 Tested-by: John Allen <john.allen@amd.com>
-Reviewed-by: Kees Cook <keescook@chromium.org>
-Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
+Tested-by: Kees Cook <keescook@chromium.org>
+---
+v8:
+ - Update commit log verbiage (Boris)
+ - Drop set_clr_bits_msrl() (Boris)
+ - Fix comments wrss->WRSS (Boris)
 
----
 v6:
- - Make set_clr_bits_msrl() avoid side affects in 'msr'
+ - Make set_clr_bits_msrl() avoid side effects in 'msr'
 
 v5:
  - Switch to EOPNOTSUPP
@@ -32,40 +38,11 @@
 v3:
  - Make wrss_control() static
  - Fix verbiage in commit log (Kees)
+---
+ arch/x86/include/uapi/asm/prctl.h |  1 +
+ arch/x86/kernel/shstk.c           | 43 ++++++++++++++++++++++++++++++-
+ 2 files changed, 43 insertions(+), 1 deletion(-)
 
-v2:
- - Add some commit log verbiage from (Dave Hansen)
-
-v1:
- - New patch.
----
- arch/x86/include/asm/msr.h        | 11 +++++++++++
- arch/x86/include/uapi/asm/prctl.h |  1 +
- arch/x86/kernel/shstk.c           | 32 ++++++++++++++++++++++++++++++-
- 3 files changed, 43 insertions(+), 1 deletion(-)
-
-diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
-index 65ec1965cd28..2d3b35c957ad 100644
---- a/arch/x86/include/asm/msr.h
-+++ b/arch/x86/include/asm/msr.h
-@@ -310,6 +310,17 @@ void msrs_free(struct msr *msrs);
- int msr_set_bit(u32 msr, u8 bit);
- int msr_clear_bit(u32 msr, u8 bit);
- 
-+/* Helper that can never get accidentally un-inlined. */
-+#define set_clr_bits_msrl(msr, set, clear)	do {	\
-+	u64 __val, __new_val, __msr = msr;		\
-+							\
-+	rdmsrl(__msr, __val);				\
-+	__new_val = (__val & ~(clear)) | (set);		\
-+							\
-+	if (__new_val != __val)				\
-+		wrmsrl(__msr, __new_val);		\
-+} while (0)
-+
- #ifdef CONFIG_SMP
- int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h);
- int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h);
 diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
 index 7dfd9dc00509..e31495668056 100644
 --- a/arch/x86/include/uapi/asm/prctl.h
@@ -78,21 +55,23 @@
  
  #endif /* _ASM_X86_PRCTL_H */
 diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
-index 0a3decab70ee..009cb3fa0ae5 100644
+index 6d2531ce661c..01b45666f1b6 100644
 --- a/arch/x86/kernel/shstk.c
 +++ b/arch/x86/kernel/shstk.c
-@@ -363,6 +363,36 @@ void shstk_free(struct task_struct *tsk)
+@@ -360,6 +360,47 @@ void shstk_free(struct task_struct *tsk)
  	unmap_shadow_stack(shstk->base, shstk->size);
  }
  
 +static int wrss_control(bool enable)
 +{
++	u64 msrval;
++
 +	if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK))
 +		return -EOPNOTSUPP;
 +
 +	/*
-+	 * 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
++	 * 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 (!features_enabled(ARCH_SHSTK_SHSTK))
@@ -103,13 +82,22 @@
 +		return 0;
 +
 +	fpregs_lock_and_load();
++	rdmsrl(MSR_IA32_U_CET, msrval);
++
 +	if (enable) {
-+		set_clr_bits_msrl(MSR_IA32_U_CET, CET_WRSS_EN, 0);
 +		features_set(ARCH_SHSTK_WRSS);
++		msrval |= CET_WRSS_EN;
 +	} else {
-+		set_clr_bits_msrl(MSR_IA32_U_CET, 0, CET_WRSS_EN);
 +		features_clr(ARCH_SHSTK_WRSS);
++		if (!(msrval & CET_WRSS_EN))
++			goto unlock;
++
++		msrval &= ~CET_WRSS_EN;
 +	}
++
++	wrmsrl(MSR_IA32_U_CET, msrval);
++
++unlock:
 +	fpregs_unlock();
 +
 +	return 0;
@@ -118,7 +106,7 @@
  static int shstk_disable(void)
  {
  	if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK))
-@@ -379,7 +409,7 @@ static int shstk_disable(void)
+@@ -376,7 +417,7 @@ static int shstk_disable(void)
  	fpregs_unlock();
  
  	shstk_free(current);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help