[PATCH] riscv: signal: protect regs->status RMW from concurrent preemption

Guobin Zhang <[email protected]>
Newsgroups org.infradead.lists.linux-riscv,org.kernel.vger.linux-kernel
Message-ID <20260807-vector_fpu_regs_status_rmw_fix-v1-1-0c16848b60db@intel.com>
__fstate_clean() performs a non-atomic read-modify-write (RMW) on
task_pt_regs(current)->status to clear the FS bits. This RMW is
unprotected - it runs with preemption enabled and IRQs on during
signal delivery (setup_rt_frame -> fstate_save) and sigreturn
(restore_fp_state -> fstate_restore).

If a reschedule IPI or timer interrupt triggers preemption between
the load and store of this RMW, __switch_to_vector() calls
riscv_v_vstate_set_restore() which modifies the VS bits of the
same regs->status. When the preempted task resumes, it stores back
the stale value captured before preemption, overwriting the VS
update and restoring VS=DIRTY while TIF_RISCV_V_DEFER_RESTORE
remains set - a combination that should never occur and leads to
vector state corruption.

The race window:

  __fstate_clean (signal path)      set_restore (schedule path)
  -----------------------------     -----------------------------
  ld  a0, regs->status  // VS=DIRTY, FS=DIRTY
                                     <- preempted (reschedule IPI)
                                     regs->status VS = INITIAL
                                     set TIF_RISCV_V_DEFER_RESTORE
                                     <- resumed
  andi a0, ~FS
  ori  a0, FS_CLEAN     // stale a0 still has VS=DIRTY
  sd   a0, regs->status // overwrites VS=INITIAL with VS=DIRTY
  -> result: VS=DIRTY + DEFER -> ANOMALY

Fix by adding preempt_disable/enable around the RMW in
__fstate_clean() and fstate_off(). riscv_v_vstate_set_restore()
has the identical hazard: it is called from __restore_v_state()
(sigreturn path) with preemption enabled, and it can race the same
way against the scheduler's own call to riscv_v_vstate_set_restore()
for the same task in __switch_to_vector(). Wrap that call site with
preempt_disable/enable as well.

Add WARN_ON_ONCE(preemptible()) to riscv_v_vstate_restore()
to catch any future unprotected callers at development time.

Signed-off-by: Guobin Zhang <[email protected]>
---
Race: regs->status is shared between the FPU (SR_FS) and vector
(SR_VS) state machines. Three call sites do a non-atomic RMW on it
while preemptible: __fstate_clean()/fstate_off() in switch_to.h, and
riscv_v_vstate_set_restore() as called from signal.c's
__restore_v_state() (sigreturn path).

Cause: if the task is preempted between the load and the store, the
scheduler's own switch_to() writes the other half of the same
register word when switching this task back in. The task then
resumes and stores its stale value, clobbering that write.

Fix: wrap each RMW with preempt_disable()/preempt_enable(). This is
sufficient because the conflicting writer only runs via an actual
context switch of this task, which preempt_disable() prevents.
local_irq_disable() is not needed: no IRQ handler touches
regs->status, so masking IRQs would add latency without closing any
extra race.

Reproduced on Spacemit K1 hardware; not reproducible under QEMU/TCG
(see commit message for the full race-window trace).
---
 arch/riscv/include/asm/switch_to.h | 4 ++++
 arch/riscv/include/asm/vector.h    | 2 ++
 arch/riscv/kernel/signal.c         | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 0e71eb82f920..fd0ceebd1c23 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -21,13 +21,17 @@ extern void __fstate_restore(struct task_struct *restore_from);
 
 static inline void __fstate_clean(struct pt_regs *regs)
 {
+	preempt_disable();
 	regs->status = (regs->status & ~SR_FS) | SR_FS_CLEAN;
+	preempt_enable();
 }
 
 static inline void fstate_off(struct task_struct *task,
 			      struct pt_regs *regs)
 {
+	preempt_disable();
 	regs->status = (regs->status & ~SR_FS) | SR_FS_OFF;
+	preempt_enable();
 }
 
 static inline void fstate_save(struct task_struct *task,
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 00cb9c0982b1..da1559bcee36 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -315,6 +315,8 @@ static inline void riscv_v_vstate_save(struct __riscv_v_ext_state *vstate,
 static inline void riscv_v_vstate_restore(struct __riscv_v_ext_state *vstate,
 					  struct pt_regs *regs)
 {
+	WARN_ON_ONCE(preemptible());
+
 	if (riscv_v_vstate_query(regs)) {
 		__riscv_v_vstate_restore(vstate, vstate->datap);
 		__riscv_v_vstate_clean(regs);
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 59784dc117e4..6f6f3315bc09 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -123,7 +123,9 @@ static long __restore_v_state(struct pt_regs *regs, void __user *sc_vec)
 	 * to avoid getting the vstate incorrectly clobbered by the
 	 *  discarded vector state.
 	 */
+	preempt_disable();
 	riscv_v_vstate_set_restore(current, regs);
+	preempt_enable();
 
 	/* Copy everything of __sc_riscv_v_state except datap. */
 	err = __copy_from_user(&current->thread.vstate, &state->v_state,

---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260807-vector_fpu_regs_status_rmw_fix-b51fa4a453d8

Best regards,
--  
Guobin Zhang <[email protected]>


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.