git: 92211458689b - main - amd64: Only update fsbase/gsbase in pcb for curthread.
John Baldwin
jhb at FreeBSD.org
Fri Mar 12 17:49:23 UTC 2021
The branch main has been updated by jhb:
URL: https://cgit.FreeBSD.org/src/commit/?id=92211458689b448cda52a659f9d192fef5a9dd50
commit 92211458689b448cda52a659f9d192fef5a9dd50
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-12 17:45:18 +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.
Reported by: gallatin
Discussed with: kib
MFC after: 1 week
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D29023
---
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