linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] KVM: nVMX: reset nested_run_pending if the vCPU is going to be reset
@ 2017-03-05 22:17 Wanpeng Li
  2017-03-06 10:40 ` David Hildenbrand
  0 siblings, 1 reply; 3+ messages in thread
From: Wanpeng Li @ 2017-03-05 22:17 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li, Dmitry Vyukov

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@google.com>
Suggested-by: Radim Krčmář <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
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);
+		}
 		break;
 	case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
 		if (!msr_info->host_initiated)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KVM: nVMX: reset nested_run_pending if the vCPU is going to be reset
  2017-03-05 22:17 [PATCH v2] KVM: nVMX: reset nested_run_pending if the vCPU is going to be reset Wanpeng Li
@ 2017-03-06 10:40 ` David Hildenbrand
  2017-03-06 12:05   ` Wanpeng Li
  0 siblings, 1 reply; 3+ messages in thread
From: David Hildenbrand @ 2017-03-06 10:40 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář, Wanpeng Li, Dmitry Vyukov

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@google.com>
> Suggested-by: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> 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()?

> +		}
>  		break;
>  	case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
>  		if (!msr_info->host_initiated)
> 


-- 
Thanks,

David

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KVM: nVMX: reset nested_run_pending if the vCPU is going to be reset
  2017-03-06 10:40 ` David Hildenbrand
@ 2017-03-06 12:05   ` Wanpeng Li
  0 siblings, 0 replies; 3+ messages in thread
From: Wanpeng Li @ 2017-03-06 12:05 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Wanpeng Li, Dmitry Vyukov

2017-03-06 18:40 GMT+08:00 David Hildenbrand <david@redhat.com>:
> 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@google.com>
>> Suggested-by: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-06 12:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-05 22:17 [PATCH v2] KVM: nVMX: reset nested_run_pending if the vCPU is going to be reset Wanpeng Li
2017-03-06 10:40 ` David Hildenbrand
2017-03-06 12:05   ` Wanpeng Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).