From nobody Tue Mar 7 16:37:45 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 4PWLhn69mWz3wLwf; Tue, 7 Mar 2023 16:37:45 +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 4PWLhn5bfKz4Gyy; Tue, 7 Mar 2023 16:37:45 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1678207065; 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=9mDKwpbkvlNMRqH+/Qy0qRQ4VoyRp90BEHEJ8aIH8UE=; b=V9LGGdI/6plgSw3UKexraQsJEQZckdOHSghHIsinWEeFqnv1EuHFA9e/I3C4fbxrHucz4e p9zH53Lo7whOIIPTmHiWeh84wN8c4mQ4r/BI7br7SOfkHTAhJ3v2kt9GYqKODTKRDUOK4S Q6CqzSMbK5d07MTjN0Op3LHtAV9lTU2IiQxvM8WiUsKwxG+sU+rvXKAbH2kYboICmEjtBS q1mAhrbeudyxAHPVg5CFTTlwLI2QA2ULS/5Nc59UjJm5v/+QE3Ana8XGKtUAJLLYxiFlAq RpaffkjCb7N0CBvbR1X/n+KRpIwYeHwbX1lnXxeRy4Mob5qyYUMh1tOmeS9ZTA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1678207065; 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=9mDKwpbkvlNMRqH+/Qy0qRQ4VoyRp90BEHEJ8aIH8UE=; b=cuUT98SUM7VUDxnuGe/D18vcMcsoPtGeigsioFiy8tNAlor05QG2AUCxR38GTHXv/qD03C m+yiKgJlw5aWNGAfIipa9zTkmCs1yO60YtbYEnif2Sce3rh+69jRh9rGs39rB/LKYQaeNz +rjEzxROjo1TDjUvB9K0qJ69aIM6i9u6DhKVrmyoE3z2z3OezKypNhePhAnZTYI+ornJ9e kWm/b7h6XeZatgRasLXUux6zlzjUaZX23PuvTpUf/HN3UiBzTE7G1Xef7giFMa/CiXS4RI OdT1dK3jZRBX4HcivBTBtrBjnwiF6ht5ctVWe/RS4syLnkKmLP0sZ16/qIDF2A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1678207065; a=rsa-sha256; cv=none; b=Qe2spW5JFjRAhUEBGFmaO3gacWJuejsXphcCFF3W8maIdlRzUO0ok57NGS2Ev0xZvC59xL Wj6snQxDtPh6EoUXm7Y06FEscNp7RTDa2G1DQ8nStj0l5T6/Y2NL5s9BKGBv98SlSEqdZl Hw8lg6ZQm/Lr9gfIn6Un6uYbaugMDqIV/kzHgOszu0q2w1XRywyOZWthrUlrFRjkyume6R G4uAg7BN6x9yD4Qjvfdy37QQQTuuEi+CpGVteQvl8RzCREVlrnBmXg6MtOPtSZ3EPFBPa0 9ujlnaiCZsFAtjp0de2jp/4IOwlAYNpxOmQPsg511zc+y3oh+yG4arHLvsxnFQ== 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 4PWLhn4d6GzrHH; Tue, 7 Mar 2023 16:37:45 +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 327GbjEx090540; Tue, 7 Mar 2023 16:37:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 327Gbj8i090539; Tue, 7 Mar 2023 16:37:45 GMT (envelope-from git) Date: Tue, 7 Mar 2023 16:37:45 GMT Message-Id: <202303071637.327Gbj8i090539@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: 96064e0924d8 - releng/13.2 - 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/releng/13.2 X-Git-Reftype: branch X-Git-Commit: 96064e0924d8e456f50252205bf3b221cb2e835c Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch releng/13.2 has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=96064e0924d8e456f50252205bf3b221cb2e835c commit 96064e0924d8e456f50252205bf3b221cb2e835c Author: Mark Johnston AuthorDate: 2021-09-25 14:15:31 +0000 Commit: Ed Maste CommitDate: 2023-03-07 16:36:02 +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 Approved by: re (delphij) Fixes: df8dd6025af8 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D32057 (cherry picked from commit ca1e447b1048b26b855d7f7fbcdad78309e4d741) (cherry picked from commit cf25fa7f31e7bda9b8aa20757659524db9c4ef97) --- 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); }