From nobody Thu Mar 2 03:12:40 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4PRx483YFnz3vkTk; Thu, 2 Mar 2023 03:12:40 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4PRx4834y5z49gT; Thu, 2 Mar 2023 03:12:40 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1677726760; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=oALiJ+P5cnb6bKrYviCjl+iZfYYMtDYZvKWkq3FZ8BY=; b=nmWqWyxlfTvwetbY+WKaRSCllBCNjkqdv1OFsn4Zi4ZpbRbJJrzwL9xbboKnaaY1Oz0/Di i5xWegfKN8QLW48M371F7fHKQ7nqKxsG6MFBMpwktL5OkYLXdzsHf171Jsa6txBCjkXlva 6MJddDXBVynpzzPi69ae6EjjH3TZEuv8Tf63Fjb1LzgTHIYNLZL4+qNi9vApcga537d6If B7KbLqSXL75qtUCNhHZjHf9nmX4i3X1jwLrQOxVqlPn1SHA9BNPD2t+qtkvDe2c1PMeiQO n3V+/RvdLQruOPJlvDK88ACDJj5/yPIA0t0FfnzLvd0GD7y7dWynSVwFft/5Pw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1677726760; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=oALiJ+P5cnb6bKrYviCjl+iZfYYMtDYZvKWkq3FZ8BY=; b=hiYapGI3W/yAjOyXMdXR08MhPsUohsOW6Rz1BQ50SYmXziOnEORY8Up5ridrj9eayxcJnL bNe9chRSj2II4GGQbXp+njO7Ohyi/dg66qb1gB8zuzuqUpRPtj+ohQuCnwcdJBVyoxb2ML 8meduAlf/HT5pXWAZvMCXSxR1BSMyRl6Lu4Tk1blwgGrxyPW0yclcK3HpaTft5lYNcaTuf m+Y7NirZbfjG9Ct/n9b3Zxgp716Rjw3/H5YbT8EhGE29/jF2VF4lyKwUoDfD98FiQlq3c6 n2PLm7YHAu8hQBZXbh4IovlJB4D3IeBcn5fep3ZVHAy6XkE4JvZ696QuxvmPZw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1677726760; a=rsa-sha256; cv=none; b=Xmj+6gektOsinqkFmDH0Ut8ht4glT2H3CeqSYOm7VSCFXpe6gqjUS5szpH+Ee8IzPCgt/J 8fPLRG6bi6JM/aPFJlsKa26MugV6KjdMKLNodnvZSLzfGYnH4fBvOOa6vEr5Qv5OQ6a2E6 emI19jQmRQ2S0Na/dQTiBnBlEolB8+qtD6TdLUuwbtsfE1SS5cN/Q3Niq0L1UyLOtemu3X TTFb5hVgsH4oD0lTHIVb8o5wym5zgggR8ts4tSiqg3GBL6Khyw13qpSzY4krwLzmZmfJoG 9L0xpM2xEvbQsP0HonE+NvDFXFi/8JyXvjA4ZY6RWvByi4y12YczXRPkWHNp0A== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4PRx4829hcz10vj; Thu, 2 Mar 2023 03:12:40 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 3223Ceol055216; Thu, 2 Mar 2023 03:12:40 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 3223CeGl055215; Thu, 2 Mar 2023 03:12:40 GMT (envelope-from git) Date: Thu, 2 Mar 2023 03:12:40 GMT Message-Id: <202303020312.3223CeGl055215@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Ed Maste Subject: git: cf25fa7f31e7 - stable/13 - amd64: Avoid copying td_frame from kernel procs List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: emaste X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: cf25fa7f31e7bda9b8aa20757659524db9c4ef97 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=cf25fa7f31e7bda9b8aa20757659524db9c4ef97 commit cf25fa7f31e7bda9b8aa20757659524db9c4ef97 Author: Mark Johnston AuthorDate: 2021-09-25 14:15:31 +0000 Commit: Ed Maste CommitDate: 2023-03-02 03:11:16 +0000 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 (cherry picked from commit ca1e447b1048b26b855d7f7fbcdad78309e4d741) --- sys/amd64/amd64/vm_machdep.c | 48 +++++++++++++++++--------------------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c index e308f4a44b3e..c9c498180c7e 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); }