Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1684827 > unrolled thread

[PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation

Started byWanpeng Li <kernellwp@gmail.com>
First post2017-07-11 09:20 +0200
Last post2017-07-12 16:00 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation Wanpeng Li <kernellwp@gmail.com> - 2017-07-11 09:20 +0200
    Re: [PATCH] KVM: VMX: Fix invalid guest state detection after  task-switch emulation Radim Krčmář <rkrcmar@redhat.com> - 2017-07-11 18:00 +0200
      Re: [PATCH] KVM: VMX: Fix invalid guest state detection after  task-switch emulation Paolo Bonzini <pbonzini@redhat.com> - 2017-07-11 18:10 +0200
        Re: [PATCH] KVM: VMX: Fix invalid guest state detection after  task-switch emulation Wanpeng Li <kernellwp@gmail.com> - 2017-07-12 16:00 +0200

#1684827 — [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation

FromWanpeng Li <kernellwp@gmail.com>
Date2017-07-11 09:20 +0200
Subject[PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation
Message-ID<u1XCF-7G3-5@gated-at.bofh.it>
From: Wanpeng Li <wanpeng.li@hotmail.com>

This can be reproduced by EPT=1, unrestricted_guest=N, emulate_invalid_state=Y 
or EPT=0, the trace of kvm-unit-tests/taskswitch2.flat is like below, it tries 
to emulate invalid guest state task-switch:

kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
kvm_emulate_insn: 42000:0:0f 0b (0x2)
kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
kvm_inj_exception: #UD (0x0)
kvm_entry: vcpu 0
kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
kvm_emulate_insn: 42000:0:0f 0b (0x2)
kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
kvm_inj_exception: #UD (0x0)
......................

It appears that the task-switch emulation updates rflags (and vm86 
flag) only after the segments are loaded, causing vmx->emulation_required 
to be set, when in fact invalid guest state emulation is not needed.

This patch fixes it by updating vmx->emulation_required after the 
rflags (and vm86 flag) is updated in task-switch emulation.

Suggested-by: Nadav Amit <nadav.amit@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f50cbfd..70270a2 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6255,6 +6255,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
 	 * TODO: What about debug traps on tss switch?
 	 *       Are we supposed to inject them and update dr6?
 	 */
+	vmx->emulation_required = emulation_required(vcpu);
 
 	return 1;
 }
-- 
2.7.4

[toc] | [next] | [standalone]


#1685172 — Re: [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation

FromRadim Krčmář <rkrcmar@redhat.com>
Date2017-07-11 18:00 +0200
SubjectRe: [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation
Message-ID<u25JU-4bX-19@gated-at.bofh.it>
In reply to#1684827
2017-07-11 00:13-0700, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> This can be reproduced by EPT=1, unrestricted_guest=N, emulate_invalid_state=Y 
> or EPT=0, the trace of kvm-unit-tests/taskswitch2.flat is like below, it tries 
> to emulate invalid guest state task-switch:
> 
> kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
> kvm_emulate_insn: 42000:0:0f 0b (0x2)
> kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
> kvm_inj_exception: #UD (0x0)
> kvm_entry: vcpu 0
> kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
> kvm_emulate_insn: 42000:0:0f 0b (0x2)
> kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
> kvm_inj_exception: #UD (0x0)
> ......................
> 
> It appears that the task-switch emulation updates rflags (and vm86 
> flag) only after the segments are loaded, causing vmx->emulation_required 
> to be set, when in fact invalid guest state emulation is not needed.
> 
> This patch fixes it by updating vmx->emulation_required after the 
> rflags (and vm86 flag) is updated in task-switch emulation.
> 
> Suggested-by: Nadav Amit <nadav.amit@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Nadav Amit <nadav.amit@gmail.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f50cbfd..70270a2 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6255,6 +6255,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
>  	 * TODO: What about debug traps on tss switch?
>  	 *       Are we supposed to inject them and update dr6?
>  	 */
> +	vmx->emulation_required = emulation_required(vcpu);

Hm, so the problem happened because changes to rflags can flip the value
of emulation_required().  I would add this line to vmx_set_rflags() to
make sure that we fixed everything, thanks.

[toc] | [prev] | [next] | [standalone]


#1685173 — Re: [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-07-11 18:10 +0200
SubjectRe: [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation
Message-ID<u25Tz-4wz-1@gated-at.bofh.it>
In reply to#1685172
On 11/07/2017 17:54, Radim Krčmář wrote:
> 2017-07-11 00:13-0700, Wanpeng Li:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> This can be reproduced by EPT=1, unrestricted_guest=N, emulate_invalid_state=Y 
>> or EPT=0, the trace of kvm-unit-tests/taskswitch2.flat is like below, it tries 
>> to emulate invalid guest state task-switch:
>>
>> kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
>> kvm_emulate_insn: 42000:0:0f 0b (0x2)
>> kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
>> kvm_inj_exception: #UD (0x0)
>> kvm_entry: vcpu 0
>> kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
>> kvm_emulate_insn: 42000:0:0f 0b (0x2)
>> kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
>> kvm_inj_exception: #UD (0x0)
>> ......................
>>
>> It appears that the task-switch emulation updates rflags (and vm86 
>> flag) only after the segments are loaded, causing vmx->emulation_required 
>> to be set, when in fact invalid guest state emulation is not needed.
>>
>> This patch fixes it by updating vmx->emulation_required after the 
>> rflags (and vm86 flag) is updated in task-switch emulation.
>>
>> Suggested-by: Nadav Amit <nadav.amit@gmail.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Nadav Amit <nadav.amit@gmail.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index f50cbfd..70270a2 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -6255,6 +6255,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
>>  	 * TODO: What about debug traps on tss switch?
>>  	 *       Are we supposed to inject them and update dr6?
>>  	 */
>> +	vmx->emulation_required = emulation_required(vcpu);
> 
> Hm, so the problem happened because changes to rflags can flip the value
> of emulation_required().  I would add this line to vmx_set_rflags() to
> make sure that we fixed everything, thanks.

Note that there is some extra complication, because emulation_required
is expensive and you'll want to run it only when EFLAGS.VM changes.

Paolo

[toc] | [prev] | [next] | [standalone]


#1685792 — Re: [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation

FromWanpeng Li <kernellwp@gmail.com>
Date2017-07-12 16:00 +0200
SubjectRe: [PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation
Message-ID<u2qlj-t8-7@gated-at.bofh.it>
In reply to#1685173
2017-07-12 0:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> On 11/07/2017 17:54, Radim Krčmář wrote:
>> 2017-07-11 00:13-0700, Wanpeng Li:
>>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>>
>>> This can be reproduced by EPT=1, unrestricted_guest=N, emulate_invalid_state=Y
>>> or EPT=0, the trace of kvm-unit-tests/taskswitch2.flat is like below, it tries
>>> to emulate invalid guest state task-switch:
>>>
>>> kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
>>> kvm_emulate_insn: 42000:0:0f 0b (0x2)
>>> kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
>>> kvm_inj_exception: #UD (0x0)
>>> kvm_entry: vcpu 0
>>> kvm_exit: reason TASK_SWITCH rip 0x0 info 40000058 0
>>> kvm_emulate_insn: 42000:0:0f 0b (0x2)
>>> kvm_emulate_insn: 42000:0:0f 0b (0x2) failed
>>> kvm_inj_exception: #UD (0x0)
>>> ......................
>>>
>>> It appears that the task-switch emulation updates rflags (and vm86
>>> flag) only after the segments are loaded, causing vmx->emulation_required
>>> to be set, when in fact invalid guest state emulation is not needed.
>>>
>>> This patch fixes it by updating vmx->emulation_required after the
>>> rflags (and vm86 flag) is updated in task-switch emulation.
>>>
>>> Suggested-by: Nadav Amit <nadav.amit@gmail.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>> Cc: Nadav Amit <nadav.amit@gmail.com>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>> ---
>>>  arch/x86/kvm/vmx.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index f50cbfd..70270a2 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -6255,6 +6255,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
>>>       * TODO: What about debug traps on tss switch?
>>>       *       Are we supposed to inject them and update dr6?
>>>       */
>>> +    vmx->emulation_required = emulation_required(vcpu);
>>
>> Hm, so the problem happened because changes to rflags can flip the value
>> of emulation_required().  I would add this line to vmx_set_rflags() to
>> make sure that we fixed everything, thanks.
>
> Note that there is some extra complication, because emulation_required
> is expensive and you'll want to run it only when EFLAGS.VM changes.

Agreed.

Regards,
Wanpeng Li

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web