Re: [PATCH 3/7] KVM: x86: Add emulation_type to not raise #UD on CPL=3 emulation failure

[Liran Alon <LIRAN.ALON@ORACLE.COM>]

On 22/12/17 17:16, Paolo Bonzini wrote:
> On 22/12/2017 02:11, Liran Alon wrote:
>> Consider the case where the CPU raises a #GP on some instruction
>> which is now intercepted by KVM. The #GP intercept will call
>> x86_emulate_instruction(). If the x86 emulator disassembly engine is
>> incomplete and therefore doesn't know how to parse the instruction
>> which caused the #GP, x86_decode_insn() will fail which will also
>> reach handle_emulation_failure(). If there is no
>> EMULTYPE_NO_UD_ON_FAIL flag, this will cause a #UD exception to be
>> queued which is not what we want.
>
> Yup, however EMULTYPE_VMWARE has filtered the opcodes, hasn't it?  So in
> this case you shouldn't fail the decoding.

In my current implementation EMULTYPE_VMWARE is considered only after 
the disassembly engine (x86_decode_insn()) has succeeded. It is true I 
could have filtered the opcodes before invoking the disassembly engine 
but that will make code a bit more complex. In addition, I didn't saw a 
lot of value in reducing the attack surface from the disassembly engine 
itself. Only from the emulation.

Therefore, I decided to make the EMULTYPE_NO_UD_ON_FAIL flag which may 
be also useful in the future for other use cases.

Regards,
-Liran

>
>> Therefore we can summarize these flags usage as follows: 1.
>> EMULTYPE_NO_UD_ON_FAIL is used to tell emulator "if you fail to
>> disassemble the instruction, I just want you to return failure. Do
>> not queue a #UD and let me decide what should be the proper
>> response".
>>
>> 2. EMULTYPE_VMWARE is indeed used to avoid making all
>> instructions that could raise #GP to reach instruction-emulation as
>> the x86 emulator is incomplete anyway and it just, as you say,
>> increase attack surface.
>>
>> Having said that, I agree the commit messages of the 2 commits
>> introducing these flags may not be indicative enough. If we agree on
>> the written above, I can fix them in v2 of this series.
>
> Yeah, that's good.  In particular it's important to note that
> EMULTYPE_VMWARE is not for correctness, only for hardening.
>
> Thanks,
>
> Paolo
>