[src/trunk]: src/sys/arch/x86/x86 Restore the lwp's fpu state, not zeros, and...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/cbb8d79b699d
branches:  trunk
changeset: 973660:cbb8d79b699d
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Jul 06 18:30:48 2020 +0000

description:
Restore the lwp's fpu state, not zeros, and leave with fpu enabled.

We need to clear the fpu state anyway because it is likely to contain
secrets at this point.  Previously we set it to zeros, and then issued
stts to disable the fpu in order to detect the mistake of further use
of the fpu in kernel.  But there must be some path I haven't identified
yet that doesn't do fpu_handle_deferred, leading to fpudna panics.

In any case, there's no benefit to restoring the fpu state twice
(once with zeros and once with the real data).  The downside is,
although this avoids spurious fpudna traps, using fpu_kern_enter in a
softint has the side effect that -- until the next userland context
switch triggering stts -- we no longer detect misuse of fpu in the
kernel in that lwp.  This will serve for now, but we should find
another way to issue clts/stts judiciously to detect such misuse.

May improve the continued symptoms of
https://mail-index.netbsd.org/current-users/2020/07/02/msg039051.html
although may not fix everything.

diffstat:

 sys/arch/x86/x86/fpu.c |  28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diffs (57 lines):

diff -r 787536fe3a81 -r cbb8d79b699d sys/arch/x86/x86/fpu.c
--- a/sys/arch/x86/x86/fpu.c    Mon Jul 06 16:24:06 2020 +0000
+++ b/sys/arch/x86/x86/fpu.c    Mon Jul 06 18:30:48 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: fpu.c,v 1.66 2020/07/06 01:08:15 riastradh Exp $       */
+/*     $NetBSD: fpu.c,v 1.67 2020/07/06 18:30:48 riastradh Exp $       */
 
 /*
  * Copyright (c) 2008, 2019 The NetBSD Foundation, Inc.  All
@@ -96,7 +96,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.66 2020/07/06 01:08:15 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.67 2020/07/06 18:30:48 riastradh Exp $");
 
 #include "opt_multiprocessor.h"
 
@@ -417,6 +417,9 @@
 fpu_kern_leave(void)
 {
        static const union savefpu zero_fpu __aligned(64);
+       const union savefpu *savefpu;
+       struct lwp *l = curlwp;
+       struct pcb *pcb;
        struct cpu_info *ci = curcpu();
        int s;
 
@@ -424,17 +427,18 @@
        KASSERT(ci->ci_kfpu_spl != -1);
 
        /*
-        * Zero the fpu registers; otherwise we might leak secrets
-        * through Spectre-class attacks to userland, even if there are
-        * no bugs in fpu state management.
+        * Restore the FPU state immediately to avoid leaking any
+        * kernel secrets, or zero it if this is a kthread.
         */
-       fpu_area_restore(&zero_fpu, x86_xsave_features);
-
-       /*
-        * Set CR0_TS again so that the kernel can't accidentally use
-        * the FPU.
-        */
-       stts();
+       if ((l->l_pflag & LP_INTR) && (l->l_switchto != NULL))
+               l = l->l_switchto;
+       if (l->l_flag & LW_SYSTEM) {
+               savefpu = &zero_fpu;
+       } else {
+               pcb = lwp_getpcb(l);
+               savefpu = &pcb->pcb_savefpu;
+       }
+       fpu_area_restore(savefpu, x86_xsave_features);
 
        s = ci->ci_kfpu_spl;
        ci->ci_kfpu_spl = -1;