rseq: Protect event mask against membarrier IPI

PATCH Applied Thomas Gleixner 4 participants 7 messages Aug 13, 2025 – Sep 13, 2025

Subsystems (2)

Stable branch Restartable sequences support

lore

Share & tools

lore

Thomas Gleixner

author

2025-08-13 15:02 UTC #0 lore raw

PATCH 1/1 rseq: Protect event mask against membarrier IPI

From: Thomas Gleixner <[email protected]>
Date: Tue, 12 Aug 2025 16:34:43 +0200

rseq_need_restart() reads and clears task::rseq_event_mask with preemption disabled to guard against the scheduler.

But membarrier() uses an IPI and sets the PREEMPT bit in the event mask from the IPI, which leaves that RMW operation unprotected.

Use guard(irq) if CONFIG_MEMBARRIER is enabled to fix that.

Fixes 2a36ab717e8f ("rseq/membarrier: Add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ")

Signed-off-by Thomas Gleixner <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc "Paul E. McKenney" <[email protected]>

Cc Boqun Feng <[email protected]>

Cc [email protected]

---
 include/linux/rseq.h |   11 ++++++++---
 kernel/rseq.c        |   10 +++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

include/linux/rseq.h +8 -3

@@ -7,6 +7,12 @@
 #include <linux/preempt.h>
 #include <linux/sched.h>
 
+#ifdef CONFIG_MEMBARRIER
+# define RSEQ_EVENT_GUARD	irq
+#else
+# define RSEQ_EVENT_GUARD	preempt
+#endif
+
 /*
  * Map the event mask on the user-space ABI enum rseq_cs_flags
  * for direct mask checks.

@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_re
 static inline void rseq_signal_deliver(struct ksignal *ksig,
 				       struct pt_regs *regs)
 {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
 	rseq_handle_notify_resume(ksig, regs);
 }

kernel/rseq.c +5 -5

@@ -342,12 +342,12 @@ static int rseq_need_restart(struct task
 
 	/*
 	 * Load and clear event mask atomically with respect to
-	 * scheduler preemption.
+	 * scheduler preemption and membarrier IPIs.
 	 */
-	preempt_disable();
-	event_mask = t->rseq_event_mask;
-	t->rseq_event_mask = 0;
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD) {
+		event_mask = t->rseq_event_mask;
+		t->rseq_event_mask = 0;
+	}
 
 	return !!event_mask;
 }

Boqun Feng

Reviewed-by on v1 1/1

2025-08-13 16:11 UTC #1 lore raw

Quoted 11 lines · Thomas Gleixner

From: Thomas Gleixner <[email protected]>
Date: Tue, 12 Aug 2025 16:34:43 +0200
rseq_need_restart() reads and clears task::rseq_event_mask with preemption disabled to guard against the scheduler.
But membarrier() uses an IPI and sets the PREEMPT bit in the event mask from the IPI, which leaves that RMW operation unprotected.
Use guard(irq) if CONFIG_MEMBARRIER is enabled to fix that.

Quoted 7 lines

Fixes 2a36ab717e8f ("rseq/membarrier: Add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ")
Signed-off-by Thomas Gleixner <[email protected]>
Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc [email protected]

Seems reasonable to me.

Reviewed-by Boqun Feng <[email protected]>

Regards,
Boqun

Quoted 52 lines

---
 include/linux/rseq.h |   11 ++++++++---
 kernel/rseq.c        |   10 +++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

include/linux/rseq.h +8 -3

@@ -7,6 +7,12 @@
 #include <linux/preempt.h>
 #include <linux/sched.h>
 
+#ifdef CONFIG_MEMBARRIER
+# define RSEQ_EVENT_GUARD	irq
+#else
+# define RSEQ_EVENT_GUARD	preempt
+#endif
+
 /*
  * Map the event mask on the user-space ABI enum rseq_cs_flags
  * for direct mask checks.

@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_re
 static inline void rseq_signal_deliver(struct ksignal *ksig,
 				       struct pt_regs *regs)
 {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
 	rseq_handle_notify_resume(ksig, regs);
 }

kernel/rseq.c +5 -5

@@ -342,12 +342,12 @@ static int rseq_need_restart(struct task
 
 	/*
 	 * Load and clear event mask atomically with respect to
-	 * scheduler preemption.
+	 * scheduler preemption and membarrier IPIs.
 	 */
-	preempt_disable();
-	event_mask = t->rseq_event_mask;
-	t->rseq_event_mask = 0;
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD) {
+		event_mask = t->rseq_event_mask;
+		t->rseq_event_mask = 0;
+	}
 
 	return !!event_mask;
 }

Mathieu Desnoyers

maintainer Reviewed-by on v1 1/1

2025-08-13 16:14 UTC #2 lore raw

In reply to Thomas Gleixner

Quoted 10 lines · Thomas Gleixner

From: Thomas Gleixner <[email protected]>
Date: Tue, 12 Aug 2025 16:34:43 +0200
rseq_need_restart() reads and clears task::rseq_event_mask with preemption disabled to guard against the scheduler.
But membarrier() uses an IPI and sets the PREEMPT bit in the event mask from the IPI, which leaves that RMW operation unprotected.
Use guard(irq) if CONFIG_MEMBARRIER is enabled to fix that.

Reviewed-by Mathieu Desnoyers <[email protected]>

Thanks!

Mathieu

Quoted 60 lines

Fixes 2a36ab717e8f ("rseq/membarrier: Add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ")

Signed-off-by Thomas Gleixner <[email protected]>

Cc Mathieu Desnoyers <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc "Paul E. McKenney" <[email protected]>

Cc Boqun Feng <[email protected]>

Cc [email protected]

---
  include/linux/rseq.h |   11 ++++++++---
  kernel/rseq.c        |   10 +++++-----
  2 files changed, 13 insertions(+), 8 deletions(-)

include/linux/rseq.h +8 -3

@@ -7,6 +7,12 @@
  #include <linux/preempt.h>
  #include <linux/sched.h>
 
+#ifdef CONFIG_MEMBARRIER
+# define RSEQ_EVENT_GUARD	irq
+#else
+# define RSEQ_EVENT_GUARD	preempt
+#endif
+
  /*
   * Map the event mask on the user-space ABI enum rseq_cs_flags
   * for direct mask checks.

@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_re
  static inline void rseq_signal_deliver(struct ksignal *ksig,
  				       struct pt_regs *regs)
  {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
  	rseq_handle_notify_resume(ksig, regs);
  }

kernel/rseq.c +5 -5

@@ -342,12 +342,12 @@ static int rseq_need_restart(struct task
 
  	/*
  	 * Load and clear event mask atomically with respect to
-	 * scheduler preemption.
+	 * scheduler preemption and membarrier IPIs.
  	 */
-	preempt_disable();
-	event_mask = t->rseq_event_mask;
-	t->rseq_event_mask = 0;
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD) {
+		event_mask = t->rseq_event_mask;
+		t->rseq_event_mask = 0;
+	}
 
  	return !!event_mask;
  }

Signature · 3 lines

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

Mathieu Desnoyers

maintainer

2025-08-20 13:23 UTC #3 lore raw

In reply to Thomas Gleixner

Quoted 26 lines · Thomas Gleixner

From: Thomas Gleixner <[email protected]>
Date: Tue, 12 Aug 2025 16:34:43 +0200
rseq_need_restart() reads and clears task::rseq_event_mask with preemption disabled to guard against the scheduler.
But membarrier() uses an IPI and sets the PREEMPT bit in the event mask from the IPI, which leaves that RMW operation unprotected.
Use guard(irq) if CONFIG_MEMBARRIER is enabled to fix that.
Fixes 2a36ab717e8f ("rseq/membarrier: Add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ")
Signed-off-by Thomas Gleixner <[email protected]>
Cc Mathieu Desnoyers <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Boqun Feng <[email protected]>
Cc [email protected]
---
  include/linux/rseq.h |   11 ++++++++---
  kernel/rseq.c        |   10 +++++-----
  2 files changed, 13 insertions(+), 8 deletions(-)

include/linux/rseq.h line 14

Quoted 8 lines

include/linux/rseq.h +5 -0

@@ -7,6 +7,12 @@
  #include <linux/preempt.h>
  #include <linux/sched.h>
 
+#ifdef CONFIG_MEMBARRIER
+# define RSEQ_EVENT_GUARD	irq
+#else
+# define RSEQ_EVENT_GUARD	preempt
+#endif

We should also update this comment in include/linux/sched.h:

        /*
         * RmW on rseq_event_mask must be performed atomically
         * with respect to preemption.
         */
        unsigned long rseq_event_mask;

to e.g.:

/*
* RmW on rseq_event_mask must be performed atomically
* with respect to preemption and membarrier IPIs. */

include/linux/rseq.h static inline void rseq_handle_notify_re · line 51

Quoted 13 lines

include/linux/rseq.h +3 -3

+
  /*
   * Map the event mask on the user-space ABI enum rseq_cs_flags
   * for direct mask checks.

@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_re
  static inline void rseq_signal_deliver(struct ksignal *ksig,
  				       struct pt_regs *regs)
  {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);

Then we have more to worry about interaction of the following rseq events with membarrier IPI:

- rseq_preempt, rseq_migrate, rseq_signal_deliver.

Both rseq_preempt and rseq_migrate are documented as only being required to be called with preempt off, not irq off.

I don't see the point in sharing the same rseq_event_mask across all of those rseq event sources.

Can we just move the event sources requiring preempt-off to their own word, and use a separate word for membarrier IPI instead ? This would allow us to partition the problem into two distinct states each protected by their respective mechanism.

Quoted 10 lines

include/linux/rseq.h +0 -0

  	rseq_handle_notify_resume(ksig, regs);
  }

kernel/rseq.c +0 -1

@@ -342,12 +342,12 @@ static int rseq_need_restart(struct task
 
  	/*
  	 * Load and clear event mask atomically with respect to
-	 * scheduler preemption.

kernel/rseq.c static int rseq_need_restart(struct task · line 350

Quoted 10 lines

kernel/rseq.c +5 -4

+	 * scheduler preemption and membarrier IPIs.
  	 */
-	preempt_disable();
-	event_mask = t->rseq_event_mask;
-	t->rseq_event_mask = 0;
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD) {
+		event_mask = t->rseq_event_mask;
+		t->rseq_event_mask = 0;
+	}

Instead we could sample both the preempt-off and the irq-off words here.

Thanks,

Mathieu

Quoted 3 lines

kernel/rseq.c +0 -0

  	return !!event_mask;
  }

Signature · 3 lines

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

Thomas Gleixner

author

2025-08-23 12:31 UTC #4 lore raw

Quoted 9 lines · Mathieu Desnoyers

On 2025-08-13 11:02, Thomas Gleixner wrote:
We should also update this comment in include/linux/sched.h:
         /*
          * RmW on rseq_event_mask must be performed atomically
          * with respect to preemption.
          */
         unsigned long rseq_event_mask;

Quoted 6 lines

to e.g.:
/*
* RmW on rseq_event_mask must be performed atomically
* with respect to preemption and membarrier IPIs. */

True.

Quoted 17 lines

include/linux/rseq.h static inline void rseq_handle_notify_re · line 51

Quoted 13 lines

include/linux/rseq.h +3 -3

+
  /*
   * Map the event mask on the user-space ABI enum rseq_cs_flags
   * for direct mask checks.

@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_re
  static inline void rseq_signal_deliver(struct ksignal *ksig,
  				       struct pt_regs *regs)
  {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);

Then we have more to worry about interaction of the following rseq events with membarrier IPI:

Quoted 4 lines

- rseq_preempt, rseq_migrate, rseq_signal_deliver.
Both rseq_preempt and rseq_migrate are documented as only being required to be called with preempt off, not irq off.

They are always invoked from the middle of the scheduler with interrupts disabled, so just the documentation is wrong.

Quoted 4 lines

Can we just move the event sources requiring preempt-off to their own word, and use a separate word for membarrier IPI instead ? This would allow us to partition the problem into two distinct states each protected by their respective mechanism.

signal delivery can just use set_bit() which is atomic vs. the IPI no?

But as I pointed out in the other series, we don't need that whole zoo of event bits at all. There is absolutely no point.

signal delivery does not need to set an event in the first place. It can just unconditionally invoke this stuff, no?

Thanks,

tglx

Mathieu Desnoyers

maintainer

2025-08-25 14:13 UTC #5 lore raw

Quoted 36 lines · Thomas Gleixner

Quoted 9 lines · Mathieu Desnoyers
On 2025-08-13 11:02, Thomas Gleixner wrote:
We should also update this comment in include/linux/sched.h:
          /*
           * RmW on rseq_event_mask must be performed atomically
           * with respect to preemption.
           */
          unsigned long rseq_event_mask;
Quoted 6 lines
to e.g.:
/*
* RmW on rseq_event_mask must be performed atomically
* with respect to preemption and membarrier IPIs. */
True.
Quoted 17 lines
include/linux/rseq.h static inline void rseq_handle_notify_re · line 51
Quoted 13 lines
include/linux/rseq.h +3 -3
+
   /*
    * Map the event mask on the user-space ABI enum rseq_cs_flags
    * for direct mask checks.
@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_re
   static inline void rseq_signal_deliver(struct ksignal *ksig,
   				       struct pt_regs *regs)
   {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
Then we have more to worry about interaction of the following rseq events with membarrier IPI:

Quoted 7 lines

Quoted 4 lines
- rseq_preempt, rseq_migrate, rseq_signal_deliver.
Both rseq_preempt and rseq_migrate are documented as only being required to be called with preempt off, not irq off.
They are always invoked from the middle of the scheduler with interrupts disabled, so just the documentation is wrong.

Quoted 7 lines

Quoted 5 lines
Can we just move the event sources requiring preempt-off to their own word, and use a separate word for membarrier IPI instead ? This would allow us to partition the problem into two distinct states each protected by their respective mechanism.
signal delivery can just use set_bit() which is atomic vs. the IPI no?

Good point!

Quoted 6 lines

But as I pointed out in the other series, we don't need that whole zoo of event bits at all. There is absolutely no point.
signal delivery does not need to set an event in the first place. It can just unconditionally invoke this stuff, no?

Yes, as long as it can take a page fault from there.

Thanks,

Mathieu

Quoted 6 lines

Thanks,
tglx

Signature · 3 lines

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

tip-bot2 for Thomas Gleixner

merge bot Applied

2025-09-13 17:58 UTC #6 lore raw

In reply to Thomas Gleixner

The following commit has been merged into the core/rseq branch of tip:

Commit-ID:     6eb350a2233100a283f882c023e5ad426d0ed63b
Gitweb:        https://git.kernel.org/tip/6eb350a2233100a283f882c023e5ad426d0ed63b
Author:        Thomas Gleixner <[email protected]>
AuthorDate:    Wed, 13 Aug 2025 17:02:30 +02:00
Committer:     Thomas Gleixner <[email protected]>
CommitterDate: Sat, 13 Sep 2025 19:51:59 +02:00

rseq: Protect event mask against membarrier IPI

rseq_need_restart() reads and clears task::rseq_event_mask with preemption disabled to guard against the scheduler.

But membarrier() uses an IPI and sets the PREEMPT bit in the event mask from the IPI, which leaves that RMW operation unprotected.

Use guard(irq) if CONFIG_MEMBARRIER is enabled to fix that.

Fixes 2a36ab717e8f ("rseq/membarrier: Add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ")

Signed-off-by Thomas Gleixner <[email protected]>

Reviewed-by Boqun Feng <[email protected]>

Reviewed-by Mathieu Desnoyers <[email protected]>

Cc [email protected]

---
 include/linux/rseq.h | 11 ++++++++---
 kernel/rseq.c        | 10 +++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

include/linux/rseq.h +8 -3

index bc8af3e..1fbeb61 100644
@@ -7,6 +7,12 @@
 #include <linux/preempt.h>
 #include <linux/sched.h>
 
+#ifdef CONFIG_MEMBARRIER
+# define RSEQ_EVENT_GUARD	irq
+#else
+# define RSEQ_EVENT_GUARD	preempt
+#endif
+
 /*
  * Map the event mask on the user-space ABI enum rseq_cs_flags
  * for direct mask checks.

@@ -41,9 +47,8 @@ static inline void rseq_handle_notify_resume(struct ksignal *ksig,
 static inline void rseq_signal_deliver(struct ksignal *ksig,
 				       struct pt_regs *regs)
 {
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD)
+		__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
 	rseq_handle_notify_resume(ksig, regs);
 }

kernel/rseq.c +5 -5

index b7a1ec3..2452b73 100644
@@ -342,12 +342,12 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags)
 
 	/*
 	 * Load and clear event mask atomically with respect to
-	 * scheduler preemption.
+	 * scheduler preemption and membarrier IPIs.
 	 */
-	preempt_disable();
-	event_mask = t->rseq_event_mask;
-	t->rseq_event_mask = 0;
-	preempt_enable();
+	scoped_guard(RSEQ_EVENT_GUARD) {
+		event_mask = t->rseq_event_mask;
+		t->rseq_event_mask = 0;
+	}
 
 	return !!event_mask;
 }