Inter-revision diff: patch 34

Comparing v5 (message) to v8 (message)

--- v5
+++ v8
@@ -1,43 +1,119 @@
-The kernel now has the main shadow stack functionality to support
-applications. Wire in the WRSS and shadow stack enable/disable functions
-into the existing shadow stack API skeleton.
+For the current shadow stack implementation, shadow stacks contents can't
+easily be provisioned with arbitrary 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 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.
+
+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)
 
-v4:
- - Remove "CET" references
+v6:
+ - Make set_clr_bits_msrl() avoid side effects in 'msr'
 
-v2:
- - Split from other patches
+v5:
+ - Switch to EOPNOTSUPP
+ - Move set_clr_bits_msrl() to patch where it is first used
+ - Commit log formatting
 
- arch/x86/kernel/shstk.c | 8 ++++++++
- 1 file changed, 8 insertions(+)
+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(-)
 
+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
++++ b/arch/x86/include/uapi/asm/prctl.h
+@@ -28,5 +28,6 @@
+ 
+ /* ARCH_SHSTK_ features bits */
+ #define ARCH_SHSTK_SHSTK		(1ULL <<  0)
++#define ARCH_SHSTK_WRSS			(1ULL <<  1)
+ 
+ #endif /* _ASM_X86_PRCTL_H */
 diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
-index 71dbb49b93cd..07142e6f05f6 100644
+index 6d2531ce661c..01b45666f1b6 100644
 --- a/arch/x86/kernel/shstk.c
 +++ b/arch/x86/kernel/shstk.c
-@@ -465,9 +465,17 @@ long shstk_prctl(struct task_struct *task, int option, unsigned long features)
- 		return -EINVAL;
+@@ -360,6 +360,47 @@ void shstk_free(struct task_struct *tsk)
+ 	unmap_shadow_stack(shstk->base, shstk->size);
+ }
  
- 	if (option == ARCH_SHSTK_DISABLE) {
-+		if (features & ARCH_SHSTK_WRSS)
-+			return wrss_control(false);
-+		if (features & ARCH_SHSTK_SHSTK)
-+			return shstk_disable();
- 		return -EINVAL;
- 	}
++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
++	 * when disabling.
++	 */
++	if (!features_enabled(ARCH_SHSTK_SHSTK))
++		return -EPERM;
++
++	/* Already enabled/disabled? */
++	if (features_enabled(ARCH_SHSTK_WRSS) == enable)
++		return 0;
++
++	fpregs_lock_and_load();
++	rdmsrl(MSR_IA32_U_CET, msrval);
++
++	if (enable) {
++		features_set(ARCH_SHSTK_WRSS);
++		msrval |= CET_WRSS_EN;
++	} else {
++		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;
++}
++
+ static int shstk_disable(void)
+ {
+ 	if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK))
+@@ -376,7 +417,7 @@ static int shstk_disable(void)
+ 	fpregs_unlock();
  
- 	/* Handle ARCH_SHSTK_ENABLE */
-+	if (features & ARCH_SHSTK_SHSTK)
-+		return shstk_setup();
-+	if (features & ARCH_SHSTK_WRSS)
-+		return wrss_control(true);
- 	return -EINVAL;
+ 	shstk_free(current);
+-	features_clr(ARCH_SHSTK_SHSTK);
++	features_clr(ARCH_SHSTK_SHSTK | ARCH_SHSTK_WRSS);
+ 
+ 	return 0;
  }
 -- 
 2.17.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help