On 10/05/2017 12:19, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>
> Reported by syzkaller:
>
> BUG: unable to handle kernel paging request at ffffffffc07f6a2e
> IP: report_bug+0x94/0x120
> PGD 348e12067
> P4D 348e12067
> PUD 348e14067
> PMD 3cbd84067
> PTE 80000003f7e87161
>
> Oops: 0003 [#1] SMP
> CPU: 2 PID: 7091 Comm: kvm_load_guest_ Tainted: G OE 4.11.0+ #8
> task: ffff92fdfb525400 task.stack: ffffbda6c3d04000
> RIP: 0010:report_bug+0x94/0x120
> RSP: 0018:ffffbda6c3d07b20 EFLAGS: 00010202
> do_trap+0x156/0x170
> do_error_trap+0xa3/0x170
> ? kvm_load_guest_fpu.part.175+0x12a/0x170 [kvm]
> ? mark_held_locks+0x79/0xa0
> ? retint_kernel+0x10/0x10
> ? trace_hardirqs_off_thunk+0x1a/0x1c
> do_invalid_op+0x20/0x30
> invalid_op+0x1e/0x30
> RIP: 0010:kvm_load_guest_fpu.part.175+0x12a/0x170 [kvm]
> ? kvm_load_guest_fpu.part.175+0x1c/0x170 [kvm]
> kvm_arch_vcpu_ioctl_run+0xed6/0x1b70 [kvm]
> kvm_vcpu_ioctl+0x384/0x780 [kvm]
> ? kvm_vcpu_ioctl+0x384/0x780 [kvm]
> ? sched_clock+0x13/0x20
> ? __do_page_fault+0x2a0/0x550
> do_vfs_ioctl+0xa4/0x700
> ? up_read+0x1f/0x40
> ? __do_page_fault+0x2a0/0x550
> SyS_ioctl+0x79/0x90
> entry_SYSCALL_64_fastpath+0x23/0xc2
>
> SDM mentioned that "The MXCSR has several reserved bits, and attempting to write
> a 1 to any of these bits will cause a general-protection exception(#GP) to be
> generated". The syzkaller forks' testcase overrides xsave area w/ random values
> and steps on the reserved bits of MXCSR register. The damaged MXCSR register
> values of guest will be restored to SSEx MXCSR register before vmentry. This
> patch fixes it by catching userspace override MXCSR register reserved bits w/
> random values and bails out immediately.
>
> Reported-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Radim should be able to merge it before -rc2.
Thanks!
Paolo
> ---
> arch/x86/kvm/x86.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 464da93..5e9e0e7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3288,11 +3288,14 @@ static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
> }
> }
>
> +#define XSAVE_MXCSR_OFFSET 24
> +
> static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
> struct kvm_xsave *guest_xsave)
> {
> u64 xstate_bv =
> *(u64 *)&guest_xsave->region[XSAVE_HDR_OFFSET / sizeof(u32)];
> + u32 mxcsr = *(u32 *)&guest_xsave->region[XSAVE_MXCSR_OFFSET / sizeof(u32)];
>
> if (boot_cpu_has(X86_FEATURE_XSAVE)) {
> /*
> @@ -3300,11 +3303,13 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
> * CPUID leaf 0xD, index 0, EDX:EAX. This is for compatibility
> * with old userspace.
> */
> - if (xstate_bv & ~kvm_supported_xcr0())
> + if (xstate_bv & ~kvm_supported_xcr0() ||
> + mxcsr & ~vcpu->arch.guest_fpu.state.xsave.i387.mxcsr_mask)
> return -EINVAL;
> load_xsave(vcpu, (u8 *)guest_xsave->region);
> } else {
> - if (xstate_bv & ~XFEATURE_MASK_FPSSE)
> + if (xstate_bv & ~XFEATURE_MASK_FPSSE ||
> + mxcsr & ~vcpu->arch.guest_fpu.state.fxsave.mxcsr_mask)
> return -EINVAL;
> memcpy(&vcpu->arch.guest_fpu.state.fxsave,
> guest_xsave->region, sizeof(struct fxregs_state));
>