Skip to contentSkip to conversation

rseq: Make rseq work with protection keys

PATCH Superseded Dmitry Vyukov 1 participant 5 messages Feb 25, 2025

Subsystems (5)

v5 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v5 cover rseq: Make rseq work with protection keys

If an application registers rseq, and ever switches to another pkey protection (such that the rseq becomes inaccessible), then any context switch will cause failure in __rseq_handle_notify_resume() attempting to read/write struct rseq and/or rseq_cs. Since context switches are asynchronous and are outside of the application control (not part of the restricted code scope), temporarily enable access to 0 (default) PKEY to read/write rseq/rseq_cs.
0 is the only PKEY supported for rseq for now. Theoretically other PKEYs can be supported, but it's unclear how/if that can work. So for now we don't support that to simplify code.

Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Aruna Ramakrishna <[email protected]>

Dmitry Vyukov (4):
pkeys: add API to switch to permissive/zero pkey register x86/signal: Use write_permissive_pkey_val() helper
rseq: Make rseq work with protection keys selftests/rseq: Add test for rseq+pkeys

 arch/x86/Kconfig                         |  1 +
 arch/x86/include/asm/pkeys.h             | 30 ++++++++
 arch/x86/include/asm/pkru.h              | 10 ++-
 arch/x86/kernel/signal.c                 |  6 +-
 include/linux/pkeys.h                    | 31 ++++++++
 kernel/rseq.c                            | 11 +++
 mm/Kconfig                               |  2 +
 tools/testing/selftests/rseq/Makefile    |  2 +-
 tools/testing/selftests/rseq/pkey_test.c | 98 ++++++++++++++++++++++++
 tools/testing/selftests/rseq/rseq.h      |  1 +
 10 files changed, 184 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/rseq/pkey_test.c

base-commit: d082ecbc71e9e0bf49883ee4afd435a77a5101b6

Signature · 1 line
2.48.1.658.g4767266eb4-goog
#1 lore raw

PATCH v5 1/4 pkeys: add API to switch to permissive/zero pkey register

The API allows to switch to permissive pkey register that allows accesses to all PKEYs, and to a value that allows access to the 0 (default) PKEY. x86 signal delivery already uses switching to permissive PKEY register value, and rseq needs to allow access to PKEY 0 while accessing struct rseq/rseq_cs.

Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Aruna Ramakrishna <[email protected]>
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

---
Changes in v5:
- Removed leftover dead code in enable_zero_pkey_val
- Clarified commit message

Changes in v4:
- Added Fixes tag

Changes in v3:
- Renamed API functions to write_permissive_pkey_val/write_pkey_val
- Added enable_zero_pkey_val for rseq
- Added Reviewed-by: Mathieu Desnoyers <[email protected]>

Changes in v2:
- Fixed typo in commit description

---
 arch/x86/Kconfig             |  1 +
 arch/x86/include/asm/pkeys.h | 30 ++++++++++++++++++++++++++++++
 arch/x86/include/asm/pkru.h  | 10 +++++++---
 include/linux/pkeys.h        | 31 +++++++++++++++++++++++++++++++
 mm/Kconfig                   |  2 ++
 5 files changed, 71 insertions(+), 3 deletions(-)
arch/x86/Kconfig +1 -0
index be2c311f5118d..43af2840d098f 100644
@@ -1881,6 +1881,7 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS
 	depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_HAS_PKEYS
+	select ARCH_HAS_PERMISSIVE_PKEY
 	help
 	  Memory Protection Keys provides a mechanism for enforcing
 	  page-based protections, but without requiring modification of the
arch/x86/include/asm/pkeys.h +30 -0
index 2e6c04d8a45b4..614099766d5f2 100644
@@ -2,6 +2,8 @@
 #ifndef _ASM_X86_PKEYS_H
 #define _ASM_X86_PKEYS_H
 
+#include "pkru.h"
+
 /*
  * If more than 16 keys are ever supported, a thorough audit
  * will be necessary to ensure that the types that store key
@@ -123,4 +125,32 @@ static inline int vma_pkey(struct vm_area_struct *vma)
 	return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
 }
 
+typedef u32 pkey_reg_t;
+
+static inline pkey_reg_t write_permissive_pkey_val(void)
+{
+	return write_pkru(0);
+}
+
+static inline pkey_reg_t enable_zero_pkey_val(void)
+{
+	u32 pkru;
+
+	if (!cpu_feature_enabled(X86_FEATURE_OSPKE))
+		return 0;
+	/*
+	 * WRPKRU is relatively expensive compared to RDPKRU,
+	 * avoid it if possible.
+	 */
+	pkru = rdpkru();
+	if ((pkru & (PKRU_AD_BIT|PKRU_WD_BIT)) != 0)
+		wrpkru(pkru & ~(PKRU_AD_BIT|PKRU_WD_BIT));
+	return pkru;
+}
+
+static inline void write_pkey_val(pkey_reg_t val)
+{
+	write_pkru(val);
+}
+
 #endif /*_ASM_X86_PKEYS_H */
arch/x86/include/asm/pkru.h +7 -3
index 74f0a2d34ffdd..b9bf9b7f2753b 100644
@@ -39,16 +39,20 @@ static inline u32 read_pkru(void)
 	return 0;
 }
 
-static inline void write_pkru(u32 pkru)
+static inline u32 write_pkru(u32 pkru)
 {
+	u32 old_pkru;
+
 	if (!cpu_feature_enabled(X86_FEATURE_OSPKE))
-		return;
+		return 0;
 	/*
 	 * WRPKRU is relatively expensive compared to RDPKRU.
 	 * Avoid WRPKRU when it would not change the value.
 	 */
-	if (pkru != rdpkru())
+	old_pkru = rdpkru();
+	if (pkru != old_pkru)
 		wrpkru(pkru);
+	return old_pkru;
 }
 
 static inline void pkru_write_default(void)
include/linux/pkeys.h +31 -0
index 86be8bf27b41b..262d60f6a15f8 100644
@@ -48,4 +48,35 @@ static inline bool arch_pkeys_enabled(void)
 
 #endif /* ! CONFIG_ARCH_HAS_PKEYS */
 
+#ifndef CONFIG_ARCH_HAS_PERMISSIVE_PKEY
+
+/*
+ * Common name for value of the register that controls access to PKEYs
+ * (called differently on different arches: PKRU, POR, AMR).
+ */
+typedef char pkey_reg_t;
+
+/*
+ * Sets PKEY access register to the most permissive value that allows
+ * accesses to all PKEYs. Returns the current value of PKEY register.
+ * Code should generally arrange switching back to the old value
+ * using write_pkey_val(old_value).
+ */
+static inline pkey_reg_t write_permissive_pkey_val(void)
+{
+	return 0;
+}
+
+/*
+ * Sets PKEY access register to a value that allows access to the 0 (default)
+ * PKEY. Returns the current value of PKEY register.
+ */
+static inline pkey_reg_t enable_zero_pkey_val(void)
+{
+	return 0;
+}
+
+static inline void write_pkey_val(pkey_reg_t val) {}
+#endif /* ! CONFIG_ARCH_HAS_PERMISSIVE_PKEY */
+
 #endif /* _LINUX_PKEYS_H */
mm/Kconfig +2 -0
index 1b501db064172..9e874f7713a2b 100644
@@ -1147,6 +1147,8 @@ config ARCH_USES_HIGH_VMA_FLAGS
 	bool
 config ARCH_HAS_PKEYS
 	bool
+config ARCH_HAS_PERMISSIVE_PKEY
+	bool
 
 config ARCH_USES_PG_ARCH_2
 	bool
Signature · 1 line
2.48.1.658.g4767266eb4-goog
#2 lore raw

PATCH v5 2/4 x86/signal: Use write_permissive_pkey_val() helper

In reply to Dmitry Vyukov

Use the new switch_to_permissive_pkey_reg() helper instead of the custom code. No functional changes intended.

Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Aruna Ramakrishna <[email protected]>

---
Changes in v3:
- restore sig_prepare_pkru with the large comment and make it call the new write_permissive_pkey_val

---
 arch/x86/kernel/signal.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
arch/x86/kernel/signal.c +2 -4
index 5f441039b5725..27a66a0697dd2 100644
@@ -28,6 +28,7 @@
 #include <linux/entry-common.h>
 #include <linux/syscalls.h>
 #include <linux/rseq.h>
+#include <linux/pkeys.h>
 
 #include <asm/processor.h>
 #include <asm/ucontext.h>
@@ -72,10 +73,7 @@ static inline int is_x32_frame(struct ksignal *ksig)
  */
 static inline u32 sig_prepare_pkru(void)
 {
-	u32 orig_pkru = read_pkru();
-
-	write_pkru(0);
-	return orig_pkru;
+	return write_permissive_pkey_val();
 }
 
 /*
Signature · 1 line
2.48.1.658.g4767266eb4-goog
#3 lore raw

PATCH v5 3/4 rseq: Make rseq work with protection keys

In reply to Dmitry Vyukov

If an application registers rseq, and ever switches to another pkey protection (such that the rseq becomes inaccessible), then any context switch will cause failure in __rseq_handle_notify_resume() attempting to read/write struct rseq and/or rseq_cs. Since context switches are asynchronous and are outside of the application control (not part of the restricted code scope), temporarily switch to pkey value that allows access to the 0 (default) PKEY.

Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Aruna Ramakrishna <[email protected]>
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

---
Changes in v4:
- Added Fixes tag

Changes in v3:
- simplify control flow to always enable access to 0 pkey

Changes in v2:
- fixed typos and reworded the comment

---
 kernel/rseq.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/rseq.c +11 -0
index 2cb16091ec0ae..9d9c976d3b78c 100644
@@ -10,6 +10,7 @@
 
 #include <linux/sched.h>
 #include <linux/uaccess.h>
+#include <linux/pkeys.h>
 #include <linux/syscalls.h>
 #include <linux/rseq.h>
 #include <linux/types.h>
@@ -402,11 +403,19 @@ static int rseq_ip_fixup(struct pt_regs *regs)
 void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs)
 {
 	struct task_struct *t = current;
+	pkey_reg_t saved_pkey;
 	int ret, sig;
 
 	if (unlikely(t->flags & PF_EXITING))
 		return;
 
+	/*
+	 * Enable access to the default (0) pkey in case the thread has
+	 * currently disabled access to it and struct rseq/rseq_cs has
+	 * 0 pkey assigned (the only supported value for now).
+	 */
+	saved_pkey = enable_zero_pkey_val();
+
 	/*
 	 * regs is NULL if and only if the caller is in a syscall path.  Skip
 	 * fixup and leave rseq_cs as is so that rseq_sycall() will detect and
@@ -419,9 +428,11 @@ void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs)
 	}
 	if (unlikely(rseq_update_cpu_node_id(t)))
 		goto error;
+	write_pkey_val(saved_pkey);
 	return;
 
 error:
+	write_pkey_val(saved_pkey);
 	sig = ksig ? ksig->sig : 0;
 	force_sigsegv(sig);
 }
Signature · 1 line
2.48.1.658.g4767266eb4-goog
#4 lore raw

PATCH v5 4/4 selftests/rseq: Add test for rseq+pkeys

In reply to Dmitry Vyukov

Add a test that ensures that PKEY-protected struct rseq_cs works and does not lead to process kills.

Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Aruna Ramakrishna <[email protected]>
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

---
Changes in v5:
- Use static for variables/functions
- Use RSEQ_READ/WRITE_ONCE instead of volatile

Changes in v4:
- Added Fixes tag

Changes in v3:
- added Acked-by: Dave Hansen <[email protected]>
- rework the test to work when only pkey 0 is supported for rseq

Changes in v2:
- change test to install protected rseq_cs instead of rseq

---
 tools/testing/selftests/rseq/Makefile    |  2 +-
 tools/testing/selftests/rseq/pkey_test.c | 98 ++++++++++++++++++++++++
 tools/testing/selftests/rseq/rseq.h      |  1 +
 3 files changed, 100 insertions(+), 1 deletion(-)
tools/testing/selftests/rseq/Makefile +1 -1
index 5a3432fceb586..9111d25fea3af 100644
@@ -16,7 +16,7 @@ OVERRIDE_TARGETS = 1
 
 TEST_GEN_PROGS = basic_test basic_percpu_ops_test basic_percpu_ops_mm_cid_test param_test \
 		param_test_benchmark param_test_compare_twice param_test_mm_cid \
-		param_test_mm_cid_benchmark param_test_mm_cid_compare_twice
+		param_test_mm_cid_benchmark param_test_mm_cid_compare_twice pkey_test
 
 TEST_GEN_PROGS_EXTENDED = librseq.so
 
tools/testing/selftests/rseq/pkey_test.c new file +98 -0

98 lines not on this page · raw message

tools/testing/selftests/rseq/rseq.h +1 -0
index ba424ce80a719..65da4a727c550 100644
@@ -8,6 +8,7 @@
 #ifndef RSEQ_H
 #define RSEQ_H
 
+#include <assert.h>
 #include <stdint.h>
 #include <stdbool.h>
 #include <pthread.h>
Signature · 1 line
2.48.1.658.g4767266eb4-goog