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.
This commit is contained in:
parent
172d2a8491
commit
9ef76e3735
|
@ -613,6 +613,12 @@ struct xcptcontext
|
|||
|
||||
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)
|
||||
|
|
|
@ -108,19 +108,14 @@ pid_t riscv_fork(const struct fork_s *context)
|
|||
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 @@ pid_t riscv_fork(const struct fork_s *context)
|
|||
/* 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 @@ pid_t riscv_fork(const struct fork_s *context)
|
|||
|
||||
/* 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 @@ pid_t riscv_fork(const struct fork_s *context)
|
|||
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().
|
||||
*/
|
||||
|
|
|
@ -161,7 +161,7 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
|
|||
|
||||
/* Set the user register context to TCB */
|
||||
|
||||
rtcb->xcp.regs = context;
|
||||
rtcb->xcp.sregs = context;
|
||||
|
||||
/* Indicate that we are in a syscall handler */
|
||||
|
||||
|
|
Loading…
Reference in New Issue