mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Guobin Zhang <guobin.zhang@intel.com>
To: Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	 Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	 Guobin Zhang <guobin.zhang@intel.com>
Subject: [PATCH] riscv: signal: protect regs->status RMW from concurrent preemption
Date: Fri, 07 Aug 2026 11:00:50 +0800	[thread overview]
Message-ID: <20260807-vector_fpu_regs_status_rmw_fix-v1-1-0c16848b60db@intel.com> (raw)

__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 <guobin.zhang@intel.com>
---
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 <guobin.zhang@intel.com>


             reply	other threads:[~2026-08-07  3:00 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07  3:00 Guobin Zhang [this message]
2026-09-25  4:33 ` Aurelien Jarno

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260807-vector_fpu_regs_status_rmw_fix-v1-1-0c16848b60db@intel.com \
    --to=guobin.zhang@intel.com \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®