Re: Core2 and v5.18-rc5 troubles

[Sean Christopherson <seanjc@google.com>]

On Tue, May 03, 2022, Zdenek Kaspar wrote:
> On Tue, 3 May 2022 16:20:31 +0000 Sean Christopherson <seanjc@google.com> wrote:
> Bisect is later on my TODO if needed... I build this kernel now on
> debian/sid (saw some compiler/binutils updates) and added KASAN as
> Maciej pointed out.
> [  229.423151] ==================================================================
> [  229.423284] BUG: KASAN: slab-out-of-bounds in fpu_copy_uabi_to_guest_fpstate+0x86/0x130

Aha!  A clue, Sherlock!  I can reproduce in a VM by hiding XSAVE from the VM; that's
why this only repros on Core2.

KASAN blames fpu_copy_uabi_to_guest_fpstate() first, but the '3' data corruption
likely comes from this line in fpu_copy_guest_fpstate_to_uabi(), as the FP+SEE
mask == 3.

		/* Make it restorable on a XSAVE enabled host */
		ustate->xsave.header.xfeatures = XFEATURE_MASK_FPSSE;

One or both of these commits is/are to blame, depending on whether we want to blame
the bad calculation, the first use of the bad calculation, or yell at both.

  be50b2065dfa ("kvm: x86: Add support for getting/setting expanded xstate buffer")
  c60427dd50ba ("x86/fpu: Add uabi_size to guest_fpu")

I believe the right way to fix this is to set the starting uABI size to KVM's
actual base uABI size, struct kvm_xsave.  I'll test the below more broadly and
send a patch.

diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index c049561f373a..99caae7e8b01 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -14,6 +14,8 @@
 #include <asm/traps.h>
 #include <asm/irq_regs.h>
 
+#include <uapi/asm/kvm.h>
+
 #include <linux/hardirq.h>
 #include <linux/pkeys.h>
 #include <linux/vmalloc.h>
@@ -247,7 +249,20 @@ bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
        gfpu->fpstate           = fpstate;
        gfpu->xfeatures         = fpu_user_cfg.default_features;
        gfpu->perm              = fpu_user_cfg.default_features;
-       gfpu->uabi_size         = fpu_user_cfg.default_size;
+
+       /*
+        * KVM sets the FP+SSE bits in the XSAVE header when copying FPU state
+        * to userspace, even when XSAVE is unsupported, so that restoring FPU
+        * state on a different CPU that does support XSAVE can cleanly load
+        * the incoming state using its natural XSAVE.  In other words, KVM's
+        * uABI size may be larger than this host's default size.  Conversely,
+        * the default size should never be larger than KVM's base uABI size;
+        * all features that can expand the uABI size must be opt-in.
+        */
+       gfpu->uabi_size         = sizeof(struct kvm_xsave);
+       if (WARN_ON_ONCE(fpu_user_cfg.default_size > gfpu->uabi_size))
+               gfpu->uabi_size = fpu_user_cfg.default_size;
+
        fpu_init_guest_permissions(gfpu);
 
        return true;