2017-03-06 18:40 GMT+08:00 David Hildenbrand <david@xxxxxxxxxx>:
> Am 05.03.2017 um 23:17 schrieb 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 <dvyukov@xxxxxxxxxx>
>> Suggested-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
>> Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
>> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>> ---
>> 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 283aa86..b4a757369 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3310,8 +3310,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>> FEATURE_CONTROL_LOCKED && !msr_info->host_initiated))
>> return 1;
>> vmx->msr_ia32_feature_control = data;
>> - if (msr_info->host_initiated && data == 0)
>> + if (msr_info->host_initiated && data == 0) {
>> + vmx->nested.nested_run_pending = 0;
>> vmx_leave_nested(vcpu);
>
> Maybe move that directly into vmx_leave_nested()?
Good point, just sent out v3.
Regards,
Wanpeng Li
>
>> + }
>> break;
>> case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
>> if (!msr_info->host_initiated)
>>
>
>
> --
> Thanks,
>
> David
