I believe this behavior would be documented in the chipset data sheet
rather than the SDM, since the chipset returns all 1s for an unclaimed
read.
Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
On Tue, Mar 7, 2017 at 8:51 AM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote:
> Before trying to do nested_get_page() in nested_vmx_merge_msr_bitmap(),
> we have already checked that the MSR bitmap address is valid (4k aligned
> and within physical limits). SDM doesn't specify what happens if the
> there is no memory mapped at the valid address, but Intel CPUs treat the
> situation as if the bitmap was configured to trap all MSRs.
>
> KVM already does that by returning false and a correct handling doesn't
> need the guest-trigerrable warning that was reported by syzkaller:
> (The warning was originally there to catch some possible bugs in nVMX.)
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
> nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
> WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
> nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 0 PID: 7832 Comm: syz-executor1 Not tainted 4.10.0+ #229
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:15 [inline]
> dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
> panic+0x1fb/0x412 kernel/panic.c:179
> __warn+0x1c4/0x1e0 kernel/panic.c:540
> warn_slowpath_null+0x2c/0x40 kernel/panic.c:583
> nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
> nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
> enter_vmx_non_root_mode arch/x86/kvm/vmx.c:10471 [inline]
> nested_vmx_run+0x6186/0xaab0 arch/x86/kvm/vmx.c:10561
> handle_vmlaunch+0x1a/0x20 arch/x86/kvm/vmx.c:7312
> vmx_handle_exit+0xfc0/0x3f00 arch/x86/kvm/vmx.c:8526
> vcpu_enter_guest arch/x86/kvm/x86.c:6982 [inline]
> vcpu_run arch/x86/kvm/x86.c:7044 [inline]
> kvm_arch_vcpu_ioctl_run+0x1418/0x4840 arch/x86/kvm/x86.c:7205
> kvm_vcpu_ioctl+0x673/0x1120 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2570
>
> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> Signed-off-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ab338581b3ec..98e82ee1e699 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9680,10 +9680,8 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
> return false;
>
> page = nested_get_page(vcpu, vmcs12->msr_bitmap);
> - if (!page) {
> - WARN_ON(1);
> + if (!page)
> return false;
> - }
> msr_bitmap_l1 = (unsigned long *)kmap(page);
>
> memset(msr_bitmap_l0, 0xff, PAGE_SIZE);
> --
> 2.12.0
>
