Re: Core2 and v5.18-rc5 troubles

[Zdenek Kaspar <zkaspar82@gmail.com>]

On Tue, 3 May 2022 23:52:14 +0000
Sean Christopherson <seanjc@google.com> wrote:

> 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;

Patch tested, everything works fine now,

Thanks, Z.