This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 622e5b26b313dfdc358566ae22b45d3e4908cfa9 Author: Ville Juven <[email protected]> AuthorDate: Wed Jul 31 15:52:15 2024 +0300 riscv/syscall: Fix fork() system call When executing fork() via a system call, the parent's stack gets corrupted by the child, as during exception return the child loads the parent's stack pointer from the context save area. This happens because the full parent stack (including what has been pushed during the system call) is copied to the child. What should be copied, is only the user stack of the parent (the kernel stack is not interesting). Fix this by only copying the parent's user stack to the child; and make the child return directly to userspace (not via dispatch_syscall). --- arch/risc-v/src/common/fork.S | 7 ++ arch/risc-v/src/common/riscv_exception_common.S | 5 +- arch/risc-v/src/common/riscv_fork.c | 95 ++++++++++++++++++++++--- arch/risc-v/src/common/riscv_swint.c | 7 +- 4 files changed, 98 insertions(+), 16 deletions(-) diff --git a/arch/risc-v/src/common/fork.S b/arch/risc-v/src/common/fork.S index 9070e93b21..9134266fec 100644 --- a/arch/risc-v/src/common/fork.S +++ b/arch/risc-v/src/common/fork.S @@ -90,6 +90,12 @@ .type up_fork, function up_fork: + +#ifdef CONFIG_LIB_SYSCALL + /* When coming via system call, everything is in place already */ + + tail riscv_fork +#else /* Create a stack frame */ addi sp, sp, -FORK_SIZEOF @@ -148,6 +154,7 @@ up_fork: REGLOAD x1, FORK_RA_OFFSET(sp) addi sp, sp, FORK_SIZEOF ret +#endif .size up_fork, .-up_fork .end diff --git a/arch/risc-v/src/common/riscv_exception_common.S b/arch/risc-v/src/common/riscv_exception_common.S index bbec8a61b2..4f7064216b 100644 --- a/arch/risc-v/src/common/riscv_exception_common.S +++ b/arch/risc-v/src/common/riscv_exception_common.S @@ -155,6 +155,7 @@ exception_common: csrr tp, CSR_SCRATCH /* Load kernel TP */ REGLOAD tp, RISCV_PERCPU_TCB(tp) + mv a7, sp /* a7 = context */ call x1, dispatch_syscall /* Dispatch the system call */ return_from_syscall: @@ -239,11 +240,7 @@ return_from_exception: load_ctx sp -#ifdef CONFIG_ARCH_KERNEL_STACK REGLOAD sp, REG_SP(sp) /* restore original sp */ -#else - addi sp, sp, XCPTCONTEXT_SIZE -#endif /* Return from exception */ diff --git a/arch/risc-v/src/common/riscv_fork.c b/arch/risc-v/src/common/riscv_fork.c index 54aeca33b5..9187cc6ab5 100644 --- a/arch/risc-v/src/common/riscv_fork.c +++ b/arch/risc-v/src/common/riscv_fork.c @@ -39,6 +39,8 @@ #include "sched/sched.h" +#ifdef CONFIG_ARCH_HAVE_FORK + /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ @@ -96,7 +98,80 @@ * ****************************************************************************/ -#ifdef CONFIG_ARCH_HAVE_FORK +#ifdef CONFIG_LIB_SYSCALL + +pid_t riscv_fork(const struct fork_s *context) +{ + struct tcb_s *parent = this_task(); + struct task_tcb_s *child; + uintptr_t newsp; + uintptr_t newtop; + uintptr_t stacktop; + uintptr_t stackutil; +#ifdef CONFIG_SCHED_THREAD_LOCAL + uintptr_t tp; +#endif + UNUSED(context); + + /* Allocate and initialize a TCB for the child task. */ + + child = nxtask_setup_fork((start_t)parent->xcp.regs[REG_RA]); + if (!child) + { + sinfo("nxtask_setup_fork failed\n"); + return (pid_t)ERROR; + } + + /* 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]; + + /* Copy the parent stack contents (overwrites child's SP and TP) */ + + newtop = (uintptr_t)child->cmn.stack_base_ptr + child->cmn.adj_stack_size; + newsp = newtop - stackutil; + +#ifdef CONFIG_SCHED_THREAD_LOCAL + /* Save child's thread pointer */ + + tp = child->cmn.xcp.regs[REG_TP]; +#endif + + /* Set up frame for context and copy the parent's user context there */ + + memcpy((void *)(newsp - XCPTCONTEXT_SIZE), + parent->xcp.regs, XCPTCONTEXT_SIZE); + + /* Save FPU */ + + riscv_savefpu(child->cmn.xcp.regs, riscv_fpuregs(&child->cmn)); + + /* Copy the parent stack contents */ + + memcpy((void *)newsp, (const void *)parent->xcp.regs[REG_SP], stackutil); + + /* Set the new register restore area to the new stack top */ + + child->cmn.xcp.regs = (void *)(newsp - XCPTCONTEXT_SIZE); + + /* Return 0 to child */ + + child->cmn.xcp.regs[REG_A0] = 0; + child->cmn.xcp.regs[REG_SP] = newsp; +#ifdef CONFIG_SCHED_THREAD_LOCAL + child->cmn.xcp.regs[REG_TP] = tp; +#endif + + /* And, finally, start the child task. On a failure, nxtask_start_fork() + * will discard the TCB by calling nxtask_abort_fork(). + */ + + return nxtask_start_fork(child); +} + +#else pid_t riscv_fork(const struct fork_s *context) { @@ -171,14 +246,19 @@ pid_t riscv_fork(const struct fork_s *context) newtop = (uintptr_t)child->cmn.stack_base_ptr + child->cmn.adj_stack_size; newsp = newtop - stackutil; - /* Set up frame for context */ + /* Set up frame for context and copy the initial context there */ memcpy((void *)(newsp - XCPTCONTEXT_SIZE), child->cmn.xcp.regs, XCPTCONTEXT_SIZE); - child->cmn.xcp.regs = (void *)(newsp - XCPTCONTEXT_SIZE); + /* Copy the parent stack contents (overwrites child's SP and TP) */ + memcpy((void *)newsp, (const void *)(uintptr_t)context->sp, stackutil); + /* Set the new register restore area to the new stack top */ + + child->cmn.xcp.regs = (void *)(newsp - XCPTCONTEXT_SIZE); + /* Was there a frame pointer in place before? */ #ifdef CONFIG_RISCV_FRAMEPOINTER @@ -246,14 +326,6 @@ pid_t riscv_fork(const struct fork_s *context) fregs[REG_FS11] = context->fs11; /* Saved register fs11 */ #endif -#ifdef CONFIG_LIB_SYSCALL - /* Forked task starts at `dispatch_syscall()`, which requires TP holding - * TCB, in this case the child's TCB is needed. - */ - - child->cmn.xcp.regs[REG_TP] = (uintptr_t)child; -#endif - /* And, finally, start the child task. On a failure, nxtask_start_fork() * will discard the TCB by calling nxtask_abort_fork(). */ @@ -261,4 +333,5 @@ pid_t riscv_fork(const struct fork_s *context) return nxtask_start_fork(child); } +#endif /* CONFIG_LIB_SYSCALL */ #endif /* CONFIG_ARCH_HAVE_FORK */ diff --git a/arch/risc-v/src/common/riscv_swint.c b/arch/risc-v/src/common/riscv_swint.c index 3cae985f96..aebeef1cb9 100644 --- a/arch/risc-v/src/common/riscv_swint.c +++ b/arch/risc-v/src/common/riscv_swint.c @@ -130,13 +130,14 @@ static uintptr_t do_syscall(unsigned int nbr, uintptr_t parm1, * A4 = parm3 * A5 = parm4 * A6 = parm5 + * A7 = context (aka SP) * ****************************************************************************/ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1, uintptr_t parm2, uintptr_t parm3, uintptr_t parm4, uintptr_t parm5, - uintptr_t parm6) + uintptr_t parm6, void *context) { register long a0 asm("a0") = (long)(nbr); register long a1 asm("a1") = (long)(parm1); @@ -157,6 +158,10 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1, return -ENOSYS; } + /* Set the user register context to TCB */ + + rtcb->xcp.regs = context; + /* Indicate that we are in a syscall handler */ rtcb->flags |= TCB_FLAG_SYSCALL;
