Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753400AbdCFMdm (ORCPT ); Mon, 6 Mar 2017 07:33:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35390 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753046AbdCFMde (ORCPT ); Mon, 6 Mar 2017 07:33:34 -0500 Subject: Re: [PATCH v3] KVM: nVMX: reset nested_run_pending if the vCPU is going to be reset To: Wanpeng Li , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <1488801808-12027-1-git-send-email-wanpeng.li@hotmail.com> Cc: Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li , Dmitry Vyukov From: David Hildenbrand Organization: Red Hat GmbH Message-ID: Date: Mon, 6 Mar 2017 13:33:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <1488801808-12027-1-git-send-email-wanpeng.li@hotmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 06 Mar 2017 12:33:34 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3378 Lines: 99 Am 06.03.2017 um 13:03 schrieb Wanpeng Li: > From: Wanpeng Li > > Reported by syzkaller: > > WARNING: CPU: 1 PID: 27742 at arch/x86/kvm/vmx.c:11029 > nested_vmx_vmexit+0x5c35/0x74d0 arch/x86/kvm/vmx.c:11029 > CPU: 1 PID: 27742 Comm: a.out 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_vmexit+0x5c35/0x74d0 arch/x86/kvm/vmx.c:11029 > vmx_leave_nested arch/x86/kvm/vmx.c:11136 [inline] > vmx_set_msr+0x1565/0x1910 arch/x86/kvm/vmx.c:3324 > kvm_set_msr+0xd4/0x170 arch/x86/kvm/x86.c:1099 > do_set_msr+0x11e/0x190 arch/x86/kvm/x86.c:1128 > __msr_io arch/x86/kvm/x86.c:2577 [inline] > msr_io+0x24b/0x450 arch/x86/kvm/x86.c:2614 > kvm_arch_vcpu_ioctl+0x35b/0x46a0 arch/x86/kvm/x86.c:3497 > kvm_vcpu_ioctl+0x232/0x1120 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2721 > vfs_ioctl fs/ioctl.c:43 [inline] > do_vfs_ioctl+0x1bf/0x1790 fs/ioctl.c:683 > SYSC_ioctl fs/ioctl.c:698 [inline] > SyS_ioctl+0x8f/0xc0 fs/ioctl.c:689 > entry_SYSCALL_64_fastpath+0x1f/0xc2 > > The syzkaller folks reported a nested_run_pending warning during userspace > clear VMX capability which is exposed to L1 before. > > The warning gets thrown while doing > > (*(uint32_t*)0x20aecfe8 = (uint32_t)0x1); > (*(uint32_t*)0x20aecfec = (uint32_t)0x0); > (*(uint32_t*)0x20aecff0 = (uint32_t)0x3a); > (*(uint32_t*)0x20aecff4 = (uint32_t)0x0); > (*(uint64_t*)0x20aecff8 = (uint64_t)0x0); > r[29] = syscall(__NR_ioctl, r[4], 0x4008ae89ul, > 0x20aecfe8ul, 0, 0, 0, 0, 0, 0); > > i.e. KVM_SET_MSR ioctl with > > struct kvm_msrs { > .nmsrs = 1, > .pad = 0, > .entries = { > {.index = MSR_IA32_FEATURE_CONTROL, > .reserved = 0, > .data = 0} > } > } > > The VMLANCH/VMRESUME emulation should be stopped since the CPU is going to > reset here. This patch resets the nested_run_pending since the CPU is going > to be reset hence there should be nothing pending. > > Reported-by: Dmitry Vyukov > Suggested-by: Radim Krčmář > Cc: Paolo Bonzini > Cc: Radim Krčmář > Cc: Dmitry Vyukov > Cc: David Hildenbrand > Signed-off-by: Wanpeng Li > --- > v2 -> v3: > * move the reset to vmx_leave_nested() > v1 -> v2: > * cleanup comments format > > arch/x86/kvm/vmx.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 3b626d6..ab33858 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -11107,8 +11107,10 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, > */ > static void vmx_leave_nested(struct kvm_vcpu *vcpu) > { > - if (is_guest_mode(vcpu)) > + if (is_guest_mode(vcpu)) { > + to_vmx(vcpu)->nested.nested_run_pending = 0; > nested_vmx_vmexit(vcpu, -1, 0, 0); > + } > free_nested(to_vmx(vcpu)); > } > > Reviewed-by: David Hildenbrand -- Thanks, David