MIME-Version: 1.0
In-Reply-To: <1513034626-25688-1-git-send-email-tianyu.lan@intel.com>
References: <1513034626-25688-1-git-send-email-tianyu.lan@intel.com>
From: Wanpeng Li <kernellwp@gmail.com>
Date: Tue, 12 Dec 2017 14:42:14 +0800
Message-ID: <CANRm+CyOQVGh-=HkX_NffP=4Ocm_c1BE8YJAT1QQX1XTbaAX8A@mail.gmail.com>
Subject: Re: [PATCH] KVM/Emulate: Mask linear address with actual address
 width in order to avoid conflict with UNMAPPED_GVA
To: Lan Tianyu <tianyu.lan@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, Radim Krcmar <rkrcmar@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "the arch/x86 maintainers" <x86@kernel.org>, kvm <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Dmitry Vyukov <dvyukov@google.com>, Jim Mattson <jmattson@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 3530
Lines: 75

2017-12-12 7:23 GMT+08:00 Lan Tianyu <tianyu.lan@intel.com>:
>
> Reported by syzkaller:
>     WARNING: CPU: 0 PID: 27962 at arch/x86/kvm/emulate.c:5631 x86_emulate_insn+0x557/0x15f0 [kvm]
>     Modules linked in: kvm_intel kvm [last unloaded: kvm]
>     CPU: 0 PID: 27962 Comm: syz-executor Tainted: G    B   W        4.15.0-rc2-next-20171208+ #32
>     Hardware name: Intel Corporation S1200SP/S1200SP, BIOS S1200SP.86B.01.03.0006.040720161253 04/07/2016
>     RIP: 0010:x86_emulate_insn+0x557/0x15f0 [kvm]
>     RSP: 0018:ffff8807234476d0 EFLAGS: 00010282
>     RAX: 0000000000000000 RBX: ffff88072d0237a0 RCX: ffffffffa0065c4d
>     RDX: 1ffff100e5a046f9 RSI: 0000000000000003 RDI: ffff88072d0237c8
>     RBP: ffff880723447728 R08: ffff88072d020000 R09: ffffffffa008d240
>     R10: 0000000000000002 R11: ffffed00e7d87db3 R12: ffff88072d0237c8
>     R13: ffff88072d023870 R14: ffff88072d0238c2 R15: ffffffffa008d080
>     FS:  00007f8a68666700(0000) GS:ffff880802200000(0000) knlGS:0000000000000000
>     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>     CR2: 000000002009506c CR3: 000000071fec4005 CR4: 00000000003626f0
>     Call Trace:
>      x86_emulate_instruction+0x3bc/0xb70 [kvm]
>      ? reexecute_instruction.part.162+0x130/0x130 [kvm]
>      vmx_handle_exit+0x46d/0x14f0 [kvm_intel]
>      ? trace_event_raw_event_kvm_entry+0xe7/0x150 [kvm]
>      ? handle_vmfunc+0x2f0/0x2f0 [kvm_intel]
>      ? wait_lapic_expire+0x25/0x270 [kvm]
>      vcpu_enter_guest+0x720/0x1ef0 [kvm]
>      ...
>
> Syzkaller tests KVM emulation code path with setting RSP 0xffffffffffffffff
> and running in 64bit non paging mode. It triggers warning that exception
> vector is > 1f(it's initialized to be 0xff in x86_emulate_instruction())
> during emulation of pop instruction. This is due to pop emulation callback
> em_pop() returns X86EMUL_PROPAGATE_FAULT while not populate exception
> vector. POP emulation code accesses RSP with linear address
> 0xffffffffffffffff and KVM mmu returns GVA as GPA during nonpaging mode.
> In this case, GPA 0xffffffffffffffff(~0) conflicts with error code
> UNMAPPED_GVA which is defined to be (~(u64)0). Caller vcpu_mmio_gva_to
> _gpa() treats the return address as error and this also causes emulator
> _read_write_onepage() returns X86EMUL_PROPAGATE_FAULT without a validated
> exception vector.
>
> This patch is to mask linear address with address width to fix such issue
> since linear address won't occupy all 64-bit even if in 5 level paging
> mode.
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: Wanpeng Li <kernellwp@gmail.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

Tested-by: Wanpeng Li <wanpeng.li@hotmail.com>

> ---
>  arch/x86/kvm/emulate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index abe74f7..9b21412 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -697,8 +697,8 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
>         *max_size = 0;
>         switch (mode) {
>         case X86EMUL_MODE_PROT64:
> -               *linear = la;
>                 va_bits = ctxt_virt_addr_bits(ctxt);
> +               *linear = la & ((1ul << va_bits) - 1);
>                 if (get_canonical(la, va_bits) != la)
>                         goto bad;
>
> --
> 1.8.3.1
>