riscv_fork.c: Fix race condition when handling parent integer registers
We need to record the parent's integer register context upon exception
entry to a separate non-volatile area. Why?
Because xcp.regs can move due to a context switch within the fork() system
call, be it either via interrupt or a synchronization point.
Fix this by adding a "sregs" area where the saved user context is placed.
The critical section within fork() is also unnecessary.
diff --git a/arch/risc-v/include/irq.h b/arch/risc-v/include/irq.h
index fb16b84..cd2665c 100644
--- a/arch/risc-v/include/irq.h
+++ b/arch/risc-v/include/irq.h
@@ -613,6 +613,12 @@
uintreg_t *regs;
+#ifdef CONFIG_LIB_SYSCALL
+ /* User integer registers upon system call entry */
+
+ uintreg_t *sregs;
+#endif
+
/* FPU register save area */
#if defined(CONFIG_ARCH_FPU) && defined(CONFIG_ARCH_LAZYFPU)
diff --git a/arch/risc-v/src/common/riscv_fork.c b/arch/risc-v/src/common/riscv_fork.c
index 6f17f8d..76a784a 100644
--- a/arch/risc-v/src/common/riscv_fork.c
+++ b/arch/risc-v/src/common/riscv_fork.c
@@ -108,19 +108,14 @@
uintptr_t newtop;
uintptr_t stacktop;
uintptr_t stackutil;
- irqstate_t flags;
#ifdef CONFIG_SCHED_THREAD_LOCAL
uintptr_t tp;
#endif
UNUSED(context);
- /* parent regs may change in irq, we should disable irq here */
-
- flags = up_irq_save();
-
/* Allocate and initialize a TCB for the child task. */
- child = nxtask_setup_fork((start_t)parent->xcp.regs[REG_RA]);
+ child = nxtask_setup_fork((start_t)parent->xcp.sregs[REG_RA]);
if (!child)
{
sinfo("nxtask_setup_fork failed\n");
@@ -130,15 +125,15 @@
/* Copy parent user stack to child */
stacktop = (uintptr_t)parent->stack_base_ptr + parent->adj_stack_size;
- DEBUGASSERT(stacktop > parent->xcp.regs[REG_SP]);
- stackutil = stacktop - parent->xcp.regs[REG_SP];
+ DEBUGASSERT(stacktop > parent->xcp.sregs[REG_SP]);
+ stackutil = stacktop - parent->xcp.sregs[REG_SP];
/* Copy goes to child's user stack top */
newtop = (uintptr_t)child->cmn.stack_base_ptr + child->cmn.adj_stack_size;
newsp = newtop - stackutil;
- memcpy((void *)newsp, (const void *)parent->xcp.regs[REG_SP], stackutil);
+ memcpy((void *)newsp, (const void *)parent->xcp.sregs[REG_SP], stackutil);
#ifdef CONFIG_SCHED_THREAD_LOCAL
/* Save child's thread pointer */
@@ -169,7 +164,7 @@
/* Copy the parent integer context (overwrites child's SP and TP) */
- memcpy(child->cmn.xcp.regs, parent->xcp.regs, XCPTCONTEXT_SIZE);
+ memcpy(child->cmn.xcp.regs, parent->xcp.sregs, XCPTCONTEXT_SIZE);
/* Save FPU */
@@ -183,8 +178,6 @@
child->cmn.xcp.regs[REG_TP] = tp;
#endif
- up_irq_restore(flags);
-
/* And, finally, start the child task. On a failure, nxtask_start_fork()
* will discard the TCB by calling nxtask_abort_fork().
*/
diff --git a/arch/risc-v/src/common/riscv_swint.c b/arch/risc-v/src/common/riscv_swint.c
index ef64f4d..9126116 100644
--- a/arch/risc-v/src/common/riscv_swint.c
+++ b/arch/risc-v/src/common/riscv_swint.c
@@ -161,7 +161,7 @@
/* Set the user register context to TCB */
- rtcb->xcp.regs = context;
+ rtcb->xcp.sregs = context;
/* Indicate that we are in a syscall handler */
