sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

riscv/syscall: Fix fork() system call

Browse Source 
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).
This commit is contained in:
Ville Juven 2024-07-31 15:52:15 +03:00 committed by Xiang Xiao
parent 2cf8ac2f63
commit 622e5b26b3
4 changed files with 98 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
arch/risc-v/src/common/fork.S
View File

@ -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

5
arch/risc-v/src/common/riscv_exception_common.S
View File

@ -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 */

95
arch/risc-v/src/common/riscv_fork.c
View File

@ -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 */

7
arch/risc-v/src/common/riscv_swint.c
View File

@ -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;