git: 57d7992b50ba - stable/13 - amd64: Only update fsbase/gsbase in pcb for curthread.

John Baldwin jhb at FreeBSD.org
Mon Mar 29 22:11:57 UTC 2021


The branch stable/13 has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=57d7992b50ba36290422bd208d45033de8946307

commit 57d7992b50ba36290422bd208d45033de8946307
Author:     John Baldwin <jhb at FreeBSD.org>
AuthorDate: 2021-03-12 17:45:18 +0000
Commit:     John Baldwin <jhb at FreeBSD.org>
CommitDate: 2021-03-29 18:09:26 +0000

    amd64: Only update fsbase/gsbase in pcb for curthread.
    
    Before the pcb is copied to the new thread during cpu_fork() and
    cpu_copy_thread(), the kernel re-reads the current register values in
    case they are stale.  This is done by setting PCB_FULL_IRET in
    pcb_flags.
    
    This works fine for user threads, but the creation of kernel processes
    and kernel threads do not follow the normal synchronization rules for
    pcb_flags.  Specifically, new kernel processes are always forked from
    thread0, not from curthread, so adjusting pcb_flags via a simple
    instruction without the LOCK prefix can race with thread0 running on
    another CPU.  Similarly, kthread_add() clones from the first thread in
    the relevant kernel process, not from curthread.  In practice, Netflix
    encountered a panic where the pcb_flags in the first kthread of the
    KTLS process were trashed due to update_pcb_bases() in
    cpu_copy_thread() running from thread0 to create one of the other KTLS
    threads racing with the first KTLS kthread calling fpu_kern_thread()
    on another CPU.  In the panicking case, the write to update pcb_flags
    in fpu_kern_thread() was lost triggering an "Unregistered use of FPU
    in kernel" panic when the first KTLS kthread later tried to use the
    FPU.
    
    Sponsored by:   Netflix
    
    (cherry picked from commit 92211458689b448cda52a659f9d192fef5a9dd50)
---
 sys/amd64/amd64/vm_machdep.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c
index 1d9eacd8a8b8..76f7f400dd9c 100644
--- a/sys/amd64/amd64/vm_machdep.c
+++ b/sys/amd64/amd64/vm_machdep.c
@@ -166,7 +166,8 @@ cpu_fork(struct thread *td1, struct proc *p2, struct thread *td2, int flags)
 
 	/* Ensure that td1's pcb is up to date. */
 	fpuexit(td1);
-	update_pcb_bases(td1->td_pcb);
+	if (td1 == curthread)
+		update_pcb_bases(td1->td_pcb);
 
 	/* Point the stack and pcb to the actual location */
 	set_top_of_stack_td(td2);
@@ -568,7 +569,8 @@ cpu_copy_thread(struct thread *td, struct thread *td0)
 	 * Those not loaded individually below get their default
 	 * values here.
 	 */
-	update_pcb_bases(td0->td_pcb);
+	if (td0 == curthread)
+		update_pcb_bases(td0->td_pcb);
 	bcopy(td0->td_pcb, pcb2, sizeof(*pcb2));
 	clear_pcb_flags(pcb2, PCB_FPUINITDONE | PCB_USERFPUINITDONE |
 	    PCB_KERNFPU);


More information about the dev-commits-src-all mailing list