diff options
author | Mark Johnston <markj@FreeBSD.org> | 2021-09-25 14:15:31 +0000 |
---|---|---|
committer | Mark Johnston <markj@FreeBSD.org> | 2021-09-25 14:18:30 +0000 |
commit | ca1e447b1048b26b855d7f7fbcdad78309e4d741 (patch) | |
tree | d1ab33620ddbc51161089e400b2687f08bb97696 | |
parent | ed8ef7ae8b05d9f73db2182b5e24b0b76b0783a2 (diff) | |
download | src-ca1e447b1048b26b855d7f7fbcdad78309e4d741.tar.gz src-ca1e447b1048b26b855d7f7fbcdad78309e4d741.zip |
amd64: Avoid copying td_frame from kernel procs
When creating a new thread, we unconditionally copy td_frame from the
creating thread. For threads which never return to user mode, this is
unnecessary since td_frame just points to the base of the stack or a
random interrupt frame.
If KASAN is configured this copying may also trigger false positives
since the td_frame region may contain poisoned stack regions. It was
not noticed before since thread0 used a dummy proc0_tf trapframe, and
kernel procs are generally created by thread0. Since commit
df8dd6025af88a99d34f549fa9591a9b8f9b75b1, though, we call
cpu_thread_alloc(&thread0) when initializing FPU state, which
reinitializes thread0.td_frame.
Work around the problem by not copying the frame unless the copying
thread came from user mode. While here, de-duplicate the copying and
remove redundant re(initialization) of td_frame.
Reported by: syzbot+2ec89312bffbf38d9aec@syzkaller.appspotmail.com
Reviewed by: kib
Fixes: df8dd6025af8
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D32057
-rw-r--r-- | sys/amd64/amd64/vm_machdep.c | 48 |
1 files changed, 18 insertions, 30 deletions
diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c index 0bfcd03d6655..c0456c1d958c 100644 --- a/sys/amd64/amd64/vm_machdep.c +++ b/sys/amd64/amd64/vm_machdep.c @@ -193,6 +193,24 @@ copy_thread(struct thread *td1, struct thread *td2) td2->td_md.md_spinlock_count = 1; td2->td_md.md_saved_flags = PSL_KERNEL | PSL_I; pmap_thread_init_invl_gen(td2); + + /* + * Copy the trap frame for the return to user mode as if from a syscall. + * This copies most of the user mode register values. Some of these + * registers are rewritten by cpu_set_upcall() and linux_set_upcall(). + */ + if ((td1->td_proc->p_flag & P_KPROC) == 0) { + bcopy(td1->td_frame, td2->td_frame, sizeof(struct trapframe)); + + /* + * If the current thread has the trap bit set (i.e. a debugger + * had single stepped the process to the system call), we need + * to clear the trap flag from the new frame. Otherwise, the new + * thread will receive a (likely unexpected) SIGTRAP when it + * executes the first instruction after returning to userland. + */ + td2->td_frame->tf_rflags &= ~PSL_T; + } } /* @@ -236,23 +254,9 @@ cpu_fork(struct thread *td1, struct proc *p2, struct thread *td2, int flags) mdp2 = &p2->p_md; bcopy(&p1->p_md, mdp2, sizeof(*mdp2)); - /* - * Copy the trap frame for the return to user mode as if from a - * syscall. This copies most of the user mode register values. - */ - td2->td_frame = (struct trapframe *)td2->td_md.md_stack_base - 1; - bcopy(td1->td_frame, td2->td_frame, sizeof(struct trapframe)); - /* Set child return values. */ p2->p_sysent->sv_set_fork_retval(td2); - /* - * If the parent process has the trap bit set (i.e. a debugger - * had single stepped the process to the system call), we need - * to clear the trap flag from the new frame. - */ - td2->td_frame->tf_rflags &= ~PSL_T; - /* As on i386, do not copy io permission bitmap. */ pcb2->pcb_tssp = NULL; @@ -602,22 +606,6 @@ cpu_copy_thread(struct thread *td, struct thread *td0) { copy_thread(td0, td); - /* - * Copy user general-purpose registers. - * - * Some of these registers are rewritten by cpu_set_upcall() - * and linux_set_upcall(). - */ - bcopy(td0->td_frame, td->td_frame, sizeof(struct trapframe)); - - /* If the current thread has the trap bit set (i.e. a debugger had - * single stepped the process to the system call), we need to clear - * the trap flag from the new frame. Otherwise, the new thread will - * receive a (likely unexpected) SIGTRAP when it executes the first - * instruction after returning to userland. - */ - td->td_frame->tf_rflags &= ~PSL_T; - set_pcb_flags_raw(td->td_pcb, PCB_FULL_IRET); } |