rseq: Make rseq work with protection keys

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 ++++++++
 include/uapi/linux/rseq.h                |  4 +
 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 +
 11 files changed, 188 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/rseq/pkey_test.c

base-commit: dd83757f6e686a2188997cb58b5975f744bb7786

Signature · 1 line

2.48.1.658.g4767266eb4-goog

Dmitry Vyukov

author

2025-02-27 14:03 UTC #1 lore raw

PATCH v6 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]>

Reviewed-by Mathieu Desnoyers <[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

Dmitry Vyukov

author

2025-02-27 14:03 UTC #2 lore raw

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

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

Dmitry Vyukov

author

2025-02-27 14:03 UTC #3 lore raw

PATCH v6 3/4 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 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 v6:
- Added a comment to struct rseq with MPK rules

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

---
 include/uapi/linux/rseq.h |  4 ++++
 kernel/rseq.c             | 11 +++++++++++
 2 files changed, 15 insertions(+)

include/uapi/linux/rseq.h +4 -0

index c233aae5eac90..019fd248cf749 100644
@@ -58,6 +58,10 @@ struct rseq_cs {
  * contained within a single cache-line.
  *
  * A single struct rseq per thread is allowed.
+ *
+ * If struct rseq or struct rseq_cs is used with Memory Protection Keys,
+ * then the assigned pkey should either be accessible whenever these structs
+ * are registered/installed, or they should be protected with pkey 0.
  */
 struct rseq {
 	/*

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

Dmitry Vyukov

author

2025-03-08 10:02 UTC #4 lore raw

Quoted 9 lines · 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.

Quoted 14 lines

Signed-off-by Dmitry Vyukov <[email protected]>
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]>
Cc [email protected]
Cc [email protected]
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

Any remaining concerns with this series?

What tree should it go into?

Quoted 79 lines

---
Changes in v6:
- Added a comment to struct rseq with MPK rules

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

---
 include/uapi/linux/rseq.h |  4 ++++
 kernel/rseq.c             | 11 +++++++++++
 2 files changed, 15 insertions(+)

include/uapi/linux/rseq.h +4 -0

index c233aae5eac90..019fd248cf749 100644
@@ -58,6 +58,10 @@ struct rseq_cs {
  * contained within a single cache-line.
  *
  * A single struct rseq per thread is allowed.
+ *
+ * If struct rseq or struct rseq_cs is used with Memory Protection Keys,
+ * then the assigned pkey should either be accessible whenever these structs
+ * are registered/installed, or they should be protected with pkey 0.
  */
 struct rseq {
        /*

kernel/rseq.c +11 -1

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);
 }
--

2.48.1.658.g4767266eb4-goog

Mathieu Desnoyers

maintainer

2025-03-10 14:31 UTC #5 lore raw

Quoted 25 lines · Dmitry Vyukov

Quoted 9 lines · 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.
Quoted 14 lines
Signed-off-by Dmitry Vyukov <[email protected]>
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]>
Cc [email protected]
Cc [email protected]
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

Quoted 3 lines

Any remaining concerns with this series?
What tree should it go into?

Usually the rseq bits go through the -tip tree.

Thanks,

Mathieu

Quoted 80 lines

---
Changes in v6:
- Added a comment to struct rseq with MPK rules

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

---
  include/uapi/linux/rseq.h |  4 ++++
  kernel/rseq.c             | 11 +++++++++++
  2 files changed, 15 insertions(+)

include/uapi/linux/rseq.h +4 -0

index c233aae5eac90..019fd248cf749 100644
@@ -58,6 +58,10 @@ struct rseq_cs {
   * contained within a single cache-line.
   *
   * A single struct rseq per thread is allowed.
+ *
+ * If struct rseq or struct rseq_cs is used with Memory Protection Keys,
+ * then the assigned pkey should either be accessible whenever these structs
+ * are registered/installed, or they should be protected with pkey 0.
   */
  struct rseq {
         /*

kernel/rseq.c +11 -1

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);
  }
--

2.48.1.658.g4767266eb4-goog

Signature · 3 lines

Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Dmitry Vyukov

author

2025-03-10 14:39 UTC #6 lore raw

Quoted 27 lines · Mathieu Desnoyers

Quoted 25 lines · Dmitry Vyukov
Quoted 23 lines · 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.
Signed-off-by Dmitry Vyukov <[email protected]>
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]>
Cc [email protected]
Cc [email protected]
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

Quoted 5 lines

Quoted 3 lines
Any remaining concerns with this series?
What tree should it go into?
Usually the rseq bits go through the -tip tree.

Thomas, Ingo, can you please take this to -tip tree? Or who would that be?

Quoted 90 lines

Thanks,

Mathieu

Quoted 72 lines

---
Changes in v6:
- Added a comment to struct rseq with MPK rules

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

---
  include/uapi/linux/rseq.h |  4 ++++
  kernel/rseq.c             | 11 +++++++++++
  2 files changed, 15 insertions(+)

include/uapi/linux/rseq.h +4 -0

index c233aae5eac90..019fd248cf749 100644
@@ -58,6 +58,10 @@ struct rseq_cs {
   * contained within a single cache-line.
   *
   * A single struct rseq per thread is allowed.
+ *
+ * If struct rseq or struct rseq_cs is used with Memory Protection Keys,
+ * then the assigned pkey should either be accessible whenever these structs
+ * are registered/installed, or they should be protected with pkey 0.
   */
  struct rseq {
         /*

kernel/rseq.c +10 -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;

Quoted 8 lines

kernel/rseq.c +1 -1

  error:
+       write_pkey_val(saved_pkey);
         sig = ksig ? ksig->sig : 0;
         force_sigsegv(sig);
  }
--

2.48.1.658.g4767266eb4-goog

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Ingo Molnar

2025-03-10 18:51 UTC #7 lore raw

Quoted 24 lines · Dmitry Vyukov

Quoted 24 lines
Quoted 24 lines
Quoted 23 lines
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.
Signed-off-by Dmitry Vyukov <[email protected]>
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]>
Cc [email protected]
Cc [email protected]
Fixes d7822b1e24f2 ("rseq: Introduce restartable sequences system call")

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Any remaining concerns with this series?
What tree should it go into?
Usually the rseq bits go through the -tip tree.
Thomas, Ingo, can you please take this to -tip tree? Or who would that be?

I was waiting whether Dave Hansen would have an opinion on this series.

Also, could you please add all the new Reviewed-by tags for the next version, plus there was still a bit of a discussion on patch #4, has that been resolved?

Thanks,

Ingo

Mathieu Desnoyers

maintainer Reviewed-by on v6 3/4

2025-03-10 14:27 UTC #8 lore raw

Quoted 10 lines · 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.
Signed-off-by Dmitry Vyukov <[email protected]>
Cc Mathieu Desnoyers <[email protected]>

Reviewed-by Mathieu Desnoyers <[email protected]>

Quoted 89 lines

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 v6:
- Added a comment to struct rseq with MPK rules

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

---
  include/uapi/linux/rseq.h |  4 ++++
  kernel/rseq.c             | 11 +++++++++++
  2 files changed, 15 insertions(+)

include/uapi/linux/rseq.h +4 -0

index c233aae5eac90..019fd248cf749 100644
@@ -58,6 +58,10 @@ struct rseq_cs {
   * contained within a single cache-line.
   *
   * A single struct rseq per thread is allowed.
+ *
+ * If struct rseq or struct rseq_cs is used with Memory Protection Keys,
+ * then the assigned pkey should either be accessible whenever these structs
+ * are registered/installed, or they should be protected with pkey 0.
   */
  struct rseq {
  	/*

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 · 3 lines

Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Dmitry Vyukov

author

2025-02-27 14:03 UTC #9 lore raw

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

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

Mathieu Desnoyers

maintainer

2025-03-10 14:30 UTC #10 lore raw

Re: [PATCH v6 4/4] selftests/rseq: Add test for rseq+pkeys

Quoted 144 lines · 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 +86 -0

new file mode 100644
index 0000000000000..cc4dd98190942
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Ensure that rseq works when rseq data is inaccessible due to PKEYs.
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+#include "rseq.h"
+#include "rseq-abi.h"
+
+static int pkey;
+static ucontext_t ucp0, ucp1;
+
+static void coroutine(void)
+{
+	int i, orig_pk0, old_pk0, old_pk1, pk0, pk1;
+	/*
+	 * When we disable access to pkey 0, globals and TLS become
+	 * inaccessible too, so we need to tread carefully.
+	 * Pkey is global so we need to copy it onto the stack.
+	 */
+	int pk = RSEQ_READ_ONCE(pkey);
+	struct timespec ts;
+
+	orig_pk0 = pkey_get(0);
+	if (pkey_set(0, PKEY_DISABLE_ACCESS))
+		err(1, "pkey_set failed");
+	old_pk0 = pkey_get(0);
+	old_pk1 = pkey_get(pk);
+
+	/*
+	 * Prevent compiler from initializing it by loading a 16-global.
+	 */
+	RSEQ_WRITE_ONCE(ts.tv_sec, 0);
+	RSEQ_WRITE_ONCE(ts.tv_nsec, 10 * 1000);
+	/*
+	 * If the kernel misbehaves, context switches in the following loop
+	 * will terminate the process with SIGSEGV.
+	 * Trigger preemption w/o accessing TLS.
+	 * Note that glibc's usleep touches errno always.
+	 */
+	for (i = 0; i < 10; i++)
+		syscall(SYS_clock_nanosleep, CLOCK_MONOTONIC, 0, &ts, NULL);
+
+	pk0 = pkey_get(0);
+	pk1 = pkey_get(pk);
+	if (pkey_set(0, orig_pk0))
+		err(1, "pkey_set failed");
+
+	/*
+	 * Ensure that the kernel has restored the previous value of pkeys
+	 * register after changing them.
+	 */
+	if (old_pk0 != pk0)
+		errx(1, "pkey 0 changed %d->%d", old_pk0, pk0);
+	if (old_pk1 != pk1)
+		errx(1, "pkey 1 changed %d->%d", old_pk1, pk1);
+
+	swapcontext(&ucp1, &ucp0);
+	abort();
+}
+
+int main(int argc, char **argv)
+{
+	pkey = pkey_alloc(0, 0);
+	if (pkey == -1) {
+		printf("[SKIP]\tKernel does not support PKEYs: %s\n",
+			strerror(errno));
+		return 0;
+	}
+
+	if (rseq_register_current_thread())
+		err(1, "rseq_register_current_thread failed");
+
+	if (getcontext(&ucp1))
+		err(1, "getcontext failed");
+	ucp1.uc_stack.ss_size = getpagesize() * 4;

tools/testing/selftests/rseq/pkey_test.c line 96

Quoted 10 lines

tools/testing/selftests/rseq/pkey_test.c new file +10 -0

+	ucp1.uc_stack.ss_sp = mmap(NULL, ucp1.uc_stack.ss_size,
+		PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+	if (ucp1.uc_stack.ss_sp == MAP_FAILED)
+		err(1, "mmap failed");
+	if (pkey_mprotect(ucp1.uc_stack.ss_sp, ucp1.uc_stack.ss_size,
+			PROT_READ | PROT_WRITE, pkey))
+		err(1, "pkey_mprotect failed");
+	makecontext(&ucp1, coroutine, 0);
+	if (swapcontext(&ucp0, &ucp1))
+		err(1, "swapcontext failed");

Should the rseq register be paired with a rseq unregister here ?

Thanks,

Mathieu

Quoted 14 lines

tools/testing/selftests/rseq/pkey_test.c new file +2 -0

+	return 0;
+}

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 · 3 lines

Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Dmitry Vyukov

author

2025-03-10 14:37 UTC #11 lore raw

Re: [PATCH v6 4/4] selftests/rseq: Add test for rseq+pkeys

Quoted 150 lines · Mathieu Desnoyers

Quoted 148 lines · 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 +90 -0

new file mode 100644
index 0000000000000..cc4dd98190942
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Ensure that rseq works when rseq data is inaccessible due to PKEYs.
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+#include "rseq.h"
+#include "rseq-abi.h"
+
+static int pkey;
+static ucontext_t ucp0, ucp1;
+
+static void coroutine(void)
+{
+     int i, orig_pk0, old_pk0, old_pk1, pk0, pk1;
+     /*
+      * When we disable access to pkey 0, globals and TLS become
+      * inaccessible too, so we need to tread carefully.
+      * Pkey is global so we need to copy it onto the stack.
+      */
+     int pk = RSEQ_READ_ONCE(pkey);
+     struct timespec ts;
+
+     orig_pk0 = pkey_get(0);
+     if (pkey_set(0, PKEY_DISABLE_ACCESS))
+             err(1, "pkey_set failed");
+     old_pk0 = pkey_get(0);
+     old_pk1 = pkey_get(pk);
+
+     /*
+      * Prevent compiler from initializing it by loading a 16-global.
+      */
+     RSEQ_WRITE_ONCE(ts.tv_sec, 0);
+     RSEQ_WRITE_ONCE(ts.tv_nsec, 10 * 1000);
+     /*
+      * If the kernel misbehaves, context switches in the following loop
+      * will terminate the process with SIGSEGV.
+      * Trigger preemption w/o accessing TLS.
+      * Note that glibc's usleep touches errno always.
+      */
+     for (i = 0; i < 10; i++)
+             syscall(SYS_clock_nanosleep, CLOCK_MONOTONIC, 0, &ts, NULL);
+
+     pk0 = pkey_get(0);
+     pk1 = pkey_get(pk);
+     if (pkey_set(0, orig_pk0))
+             err(1, "pkey_set failed");
+
+     /*
+      * Ensure that the kernel has restored the previous value of pkeys
+      * register after changing them.
+      */
+     if (old_pk0 != pk0)
+             errx(1, "pkey 0 changed %d->%d", old_pk0, pk0);
+     if (old_pk1 != pk1)
+             errx(1, "pkey 1 changed %d->%d", old_pk1, pk1);
+
+     swapcontext(&ucp1, &ucp0);
+     abort();
+}
+
+int main(int argc, char **argv)
+{
+     pkey = pkey_alloc(0, 0);
+     if (pkey == -1) {
+             printf("[SKIP]\tKernel does not support PKEYs: %s\n",
+                     strerror(errno));
+             return 0;
+     }
+
+     if (rseq_register_current_thread())
+             err(1, "rseq_register_current_thread failed");
+
+     if (getcontext(&ucp1))
+             err(1, "getcontext failed");
+     ucp1.uc_stack.ss_size = getpagesize() * 4;
+     ucp1.uc_stack.ss_sp = mmap(NULL, ucp1.uc_stack.ss_size,
+             PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+     if (ucp1.uc_stack.ss_sp == MAP_FAILED)
+             err(1, "mmap failed");

Quoted 8 lines

tools/testing/selftests/rseq/pkey_test.c line 96

Quoted 6 lines

tools/testing/selftests/rseq/pkey_test.c new file +6 -0

+     if (pkey_mprotect(ucp1.uc_stack.ss_sp, ucp1.uc_stack.ss_size,
+                     PROT_READ | PROT_WRITE, pkey))
+             err(1, "pkey_mprotect failed");
+     makecontext(&ucp1, coroutine, 0);
+     if (swapcontext(&ucp0, &ucp1))
+             err(1, "swapcontext failed");

Should the rseq register be paired with a rseq unregister here ?

I dunno. It's necessary for this test and in general. Tcmalloc won't unregister on thread exit. Even for a libc it may be a bad idea due to signals.

Quoted 24 lines

Thanks,
Mathieu
Quoted 7 lines
tools/testing/selftests/rseq/pkey_test.c new file +2 -0
+     return 0;
+}
tools/testing/selftests/rseq/rseq.h +0 -0
index ba424ce80a719..65da4a727c550 100644
tools/testing/selftests/rseq/rseq.h line 14
Quoted 7 lines
tools/testing/selftests/rseq/rseq.h +1 -0
@@ -8,6 +8,7 @@
  #ifndef RSEQ_H
  #define RSEQ_H
 
+#include <assert.h>
  #include <stdint.h>
  #include <stdbool.h>
  #include <pthread.h>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Mathieu Desnoyers

maintainer

2025-03-10 14:38 UTC #12 lore raw

Re: [PATCH v6 4/4] selftests/rseq: Add test for rseq+pkeys

Quoted 161 lines · Dmitry Vyukov

Quoted 150 lines · Mathieu Desnoyers

Quoted 148 lines · 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 +90 -0

new file mode 100644
index 0000000000000..cc4dd98190942
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Ensure that rseq works when rseq data is inaccessible due to PKEYs.
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+#include "rseq.h"
+#include "rseq-abi.h"
+
+static int pkey;
+static ucontext_t ucp0, ucp1;
+
+static void coroutine(void)
+{
+     int i, orig_pk0, old_pk0, old_pk1, pk0, pk1;
+     /*
+      * When we disable access to pkey 0, globals and TLS become
+      * inaccessible too, so we need to tread carefully.
+      * Pkey is global so we need to copy it onto the stack.
+      */
+     int pk = RSEQ_READ_ONCE(pkey);
+     struct timespec ts;
+
+     orig_pk0 = pkey_get(0);
+     if (pkey_set(0, PKEY_DISABLE_ACCESS))
+             err(1, "pkey_set failed");
+     old_pk0 = pkey_get(0);
+     old_pk1 = pkey_get(pk);
+
+     /*
+      * Prevent compiler from initializing it by loading a 16-global.
+      */
+     RSEQ_WRITE_ONCE(ts.tv_sec, 0);
+     RSEQ_WRITE_ONCE(ts.tv_nsec, 10 * 1000);
+     /*
+      * If the kernel misbehaves, context switches in the following loop
+      * will terminate the process with SIGSEGV.
+      * Trigger preemption w/o accessing TLS.
+      * Note that glibc's usleep touches errno always.
+      */
+     for (i = 0; i < 10; i++)
+             syscall(SYS_clock_nanosleep, CLOCK_MONOTONIC, 0, &ts, NULL);
+
+     pk0 = pkey_get(0);
+     pk1 = pkey_get(pk);
+     if (pkey_set(0, orig_pk0))
+             err(1, "pkey_set failed");
+
+     /*
+      * Ensure that the kernel has restored the previous value of pkeys
+      * register after changing them.
+      */
+     if (old_pk0 != pk0)
+             errx(1, "pkey 0 changed %d->%d", old_pk0, pk0);
+     if (old_pk1 != pk1)
+             errx(1, "pkey 1 changed %d->%d", old_pk1, pk1);
+
+     swapcontext(&ucp1, &ucp0);
+     abort();
+}
+
+int main(int argc, char **argv)
+{
+     pkey = pkey_alloc(0, 0);
+     if (pkey == -1) {
+             printf("[SKIP]\tKernel does not support PKEYs: %s\n",
+                     strerror(errno));
+             return 0;
+     }
+
+     if (rseq_register_current_thread())
+             err(1, "rseq_register_current_thread failed");
+
+     if (getcontext(&ucp1))
+             err(1, "getcontext failed");
+     ucp1.uc_stack.ss_size = getpagesize() * 4;
+     ucp1.uc_stack.ss_sp = mmap(NULL, ucp1.uc_stack.ss_size,
+             PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+     if (ucp1.uc_stack.ss_sp == MAP_FAILED)
+             err(1, "mmap failed");

Quoted 8 lines

tools/testing/selftests/rseq/pkey_test.c line 96

Quoted 6 lines

tools/testing/selftests/rseq/pkey_test.c new file +6 -0

+     if (pkey_mprotect(ucp1.uc_stack.ss_sp, ucp1.uc_stack.ss_size,
+                     PROT_READ | PROT_WRITE, pkey))
+             err(1, "pkey_mprotect failed");
+     makecontext(&ucp1, coroutine, 0);
+     if (swapcontext(&ucp0, &ucp1))
+             err(1, "swapcontext failed");

Should the rseq register be paired with a rseq unregister here ?

Quoted 3 lines

I dunno. It's necessary for this test and in general. Tcmalloc won't unregister on thread exit. Even for a libc it may be a bad idea due to signals.

The rseq register/unregister is only for the case where libc does not support rseq. But if you want to use rseq_register_current_thread(), then you'll want to pair it with unregister.

Thanks,

Mathieu

Quoted 25 lines

Quoted 25 lines
Thanks,
Mathieu
Quoted 7 lines
tools/testing/selftests/rseq/pkey_test.c new file +2 -0
+     return 0;
+}
tools/testing/selftests/rseq/rseq.h +0 -0
index ba424ce80a719..65da4a727c550 100644
tools/testing/selftests/rseq/rseq.h line 14
Quoted 7 lines
tools/testing/selftests/rseq/rseq.h +1 -0
@@ -8,6 +8,7 @@
   #ifndef RSEQ_H
   #define RSEQ_H
 
+#include <assert.h>
   #include <stdint.h>
   #include <stdbool.h>
   #include <pthread.h>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Signature · 3 lines

Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Dmitry Vyukov

author

2025-03-10 14:43 UTC #13 lore raw

Re: [PATCH v6 4/4] selftests/rseq: Add test for rseq+pkeys

Quoted 163 lines · Mathieu Desnoyers

Quoted 161 lines · Dmitry Vyukov

Quoted 158 lines · Mathieu Desnoyers

Quoted 144 lines · 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 +86 -0

new file mode 100644
index 0000000000000..cc4dd98190942
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Ensure that rseq works when rseq data is inaccessible due to PKEYs.
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+#include "rseq.h"
+#include "rseq-abi.h"
+
+static int pkey;
+static ucontext_t ucp0, ucp1;
+
+static void coroutine(void)
+{
+     int i, orig_pk0, old_pk0, old_pk1, pk0, pk1;
+     /*
+      * When we disable access to pkey 0, globals and TLS become
+      * inaccessible too, so we need to tread carefully.
+      * Pkey is global so we need to copy it onto the stack.
+      */
+     int pk = RSEQ_READ_ONCE(pkey);
+     struct timespec ts;
+
+     orig_pk0 = pkey_get(0);
+     if (pkey_set(0, PKEY_DISABLE_ACCESS))
+             err(1, "pkey_set failed");
+     old_pk0 = pkey_get(0);
+     old_pk1 = pkey_get(pk);
+
+     /*
+      * Prevent compiler from initializing it by loading a 16-global.
+      */
+     RSEQ_WRITE_ONCE(ts.tv_sec, 0);
+     RSEQ_WRITE_ONCE(ts.tv_nsec, 10 * 1000);
+     /*
+      * If the kernel misbehaves, context switches in the following loop
+      * will terminate the process with SIGSEGV.
+      * Trigger preemption w/o accessing TLS.
+      * Note that glibc's usleep touches errno always.
+      */
+     for (i = 0; i < 10; i++)
+             syscall(SYS_clock_nanosleep, CLOCK_MONOTONIC, 0, &ts, NULL);
+
+     pk0 = pkey_get(0);
+     pk1 = pkey_get(pk);
+     if (pkey_set(0, orig_pk0))
+             err(1, "pkey_set failed");
+
+     /*
+      * Ensure that the kernel has restored the previous value of pkeys
+      * register after changing them.
+      */
+     if (old_pk0 != pk0)
+             errx(1, "pkey 0 changed %d->%d", old_pk0, pk0);
+     if (old_pk1 != pk1)
+             errx(1, "pkey 1 changed %d->%d", old_pk1, pk1);
+
+     swapcontext(&ucp1, &ucp0);
+     abort();
+}
+
+int main(int argc, char **argv)
+{
+     pkey = pkey_alloc(0, 0);
+     if (pkey == -1) {
+             printf("[SKIP]\tKernel does not support PKEYs: %s\n",
+                     strerror(errno));
+             return 0;
+     }
+
+     if (rseq_register_current_thread())
+             err(1, "rseq_register_current_thread failed");
+
+     if (getcontext(&ucp1))
+             err(1, "getcontext failed");
+     ucp1.uc_stack.ss_size = getpagesize() * 4;

tools/testing/selftests/rseq/pkey_test.c line 96

Quoted 10 lines

tools/testing/selftests/rseq/pkey_test.c new file +10 -0

+     ucp1.uc_stack.ss_sp = mmap(NULL, ucp1.uc_stack.ss_size,
+             PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+     if (ucp1.uc_stack.ss_sp == MAP_FAILED)
+             err(1, "mmap failed");
+     if (pkey_mprotect(ucp1.uc_stack.ss_sp, ucp1.uc_stack.ss_size,
+                     PROT_READ | PROT_WRITE, pkey))
+             err(1, "pkey_mprotect failed");
+     makecontext(&ucp1, coroutine, 0);
+     if (swapcontext(&ucp0, &ucp1))
+             err(1, "swapcontext failed");

Should the rseq register be paired with a rseq unregister here ?

Quoted 7 lines

Quoted 3 lines
I dunno. It's necessary for this test and in general. Tcmalloc won't unregister on thread exit. Even for a libc it may be a bad idea due to signals.
The rseq register/unregister is only for the case where libc does not support rseq. But if you want to use rseq_register_current_thread(), then you'll want to pair it with unregister.

Why? Isn't it better to have tests more realitic?

Quoted 35 lines

Thanks,
Mathieu
Quoted 15 lines
Quoted 15 lines
Thanks,
Mathieu
Quoted 10 lines
tools/testing/selftests/rseq/pkey_test.c new file +2 -0
+     return 0;
+}
tools/testing/selftests/rseq/rseq.h +0 -0
index ba424ce80a719..65da4a727c550 100644
@@ -8,6 +8,7 @@
   #ifndef RSEQ_H
   #define RSEQ_H
 
Quoted 10 lines
Quoted 10 lines
tools/testing/selftests/rseq/rseq.h line 14
Quoted 4 lines
tools/testing/selftests/rseq/rseq.h +1 -0
+#include <assert.h>
   #include <stdint.h>
   #include <stdbool.h>
   #include <pthread.h>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Mathieu Desnoyers

maintainer

2025-03-10 15:41 UTC #14 lore raw

Re: [PATCH v6 4/4] selftests/rseq: Add test for rseq+pkeys

Quoted 169 lines · Dmitry Vyukov

Quoted 167 lines · Mathieu Desnoyers

Quoted 164 lines · Dmitry Vyukov

Quoted 150 lines · Mathieu Desnoyers

Quoted 148 lines · 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]>