On 18/12/2017 10:45, Liran Alon wrote:
> Next commits are going introduce support for accessing VMware backdoor
> ports even though guest's TSS I/O permissions bitmap doesn't allow
> access. This mimic VMware hypervisor behavior.
>
> In order to support this, next commits will change VMX/SVM to
> intercept #GP which was raised by such access and handle it by calling
> the x86 emulator to emulate instruction. Since commit "KVM: x86:
> Always allow access to VMware backdoor I/O ports", the x86 emulator
> handles access to these I/O ports by not checking these ports against
> the TSS I/O permission bitmap.
>
> It turns out that the x86 emulator is incomplete and therefore
> certain instructions that can cause #GP cannot be emulated.
> Such an example is "INT x" (opcode 0xcd) which reach emulate_int()
> which can only emulate instruction if vCPU is in real-mode.
>
> In those cases, we would like the #GP intercept to just forward #GP
> as-is to guest as if there was no intercept to begin with.
> However, current emulator code always queue #UD exception in case
> emulator fails which is not what is wanted in this flow.
>
> This commit address this issue by adding a new emulation_type flag
> that will allow the #GP intercept handler to specify it wish to just
> be aware of when instruction emulation fails and doesn't want #UD
> exception to be queued.
>
> Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@xxxxxxxxxx>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> ---
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/x86.c | 12 ++++++++----
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 516798431328..2b7ea1ac4f86 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1168,6 +1168,7 @@ enum emulation_result {
> #define EMULTYPE_SKIP (1 << 2)
> #define EMULTYPE_RETRY (1 << 3)
> #define EMULTYPE_NO_REEXECUTE (1 << 4)
> +#define EMULTYPE_NO_UD_ON_FAIL (1 << 5)
> int x86_emulate_instruction(struct kvm_vcpu *vcpu, unsigned long cr2,
> int emulation_type, void *insn, int insn_len);
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5fef09674de1..8fd2d3e1bcd4 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5425,7 +5425,7 @@ int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip)
> }
> EXPORT_SYMBOL_GPL(kvm_inject_realmode_interrupt);
>
> -static int handle_emulation_failure(struct kvm_vcpu *vcpu)
> +static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
> {
> int r = EMULATE_DONE;
>
> @@ -5437,7 +5437,11 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu)
> vcpu->run->internal.ndata = 0;
> r = EMULATE_USER_EXIT;
> }
> - kvm_queue_exception(vcpu, UD_VECTOR);
> +
> + if (emulation_type & EMULTYPE_NO_UD_ON_FAIL)
> + r = EMULATE_FAIL;
There seems to be some overlap between EMULTYPE_VMWARE and
EMULTYPE_NO_UD_ON_FAIL.
Even if you don't specify EMULTYPE_VMWARE, the emulation could fail, but
there should have been no writeback and injecting the original #GP
exception should be safe. Maybe should EMULTYPE_NO_UD_ON_FAIL return
straight away, skipping even the user-mode exit.
On the other hand, you may want to have EMULTYPE_VMWARE so as to reduce
the attack surface from the emulator (as the emulator would then be very
easy to trigger, just by executing an instruction that causes #GP). In
that case, however, emulation of the {in,out}{s,} instructions shouldn't
fail and you shouldn't need EMULTYPE_NO_UD_ON_FAIL.
Paolo
> + else
> + kvm_queue_exception(vcpu, UD_VECTOR);
>
> return r;
> }
> @@ -5731,7 +5735,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
> return EMULATE_DONE;
> if (emulation_type & EMULTYPE_SKIP)
> return EMULATE_FAIL;
> - return handle_emulation_failure(vcpu);
> + return handle_emulation_failure(vcpu, emulation_type);
> }
> }
>
> @@ -5766,7 +5770,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
> emulation_type))
> return EMULATE_DONE;
>
> - return handle_emulation_failure(vcpu);
> + return handle_emulation_failure(vcpu, emulation_type);
> }
>
> if (ctxt->have_exception) {
>
