On Tue, 13 Aug 2019 11:20:45 +0200, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> On 09/08/19 18:00, Adalbert Lazăr wrote:
> > From: Mihai Donțu <mdontu@xxxxxxxxxxxxxxx>
> >
> > It can happened for us to end up emulating the VMCALL instruction as a
> > result of the handling of an EPT write fault. In this situation, the
> > emulator will try to unconditionally patch the correct hypercall opcode
> > bytes using emulator_write_emulated(). However, this last call uses the
> > fault GPA (if available) or walks the guest page tables at RIP,
> > otherwise. The trouble begins when using KVMI, when we forbid the use of
> > the fault GPA and fallback to the guest pt walk: in Windows (8.1 and
> > newer) the page that we try to write into is marked read-execute and as
> > such emulator_write_emulated() fails and we inject a write #PF, leading
> > to a guest crash.
> >
> > The fix is rather simple: check the existing instruction bytes before
> > doing the patching. This does not change the normal KVM behaviour, but
> > does help when using KVMI as we no longer inject a write #PF.
>
> Fixing the hypercall is just an optimization. Can we just hush and
> return to the guest if emulator_write_emulated returns
> X86EMUL_PROPAGATE_FAULT?
>
> Paolo
Something like this?
err = emulator_write_emulated(...);
if (err == X86EMUL_PROPAGATE_FAULT)
err = X86EMUL_CONTINUE;
return err;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 04b1d2916a0a..965c4f0108eb 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -7363,16 +7363,33 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > }
> > EXPORT_SYMBOL_GPL(kvm_emulate_hypercall);
> >
> > +#define KVM_HYPERCALL_INSN_LEN 3
> > +
> > static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
> > {
> > + int err;
> > struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
> > - char instruction[3];
> > + char buf[KVM_HYPERCALL_INSN_LEN];
> > + char instruction[KVM_HYPERCALL_INSN_LEN];
> > unsigned long rip = kvm_rip_read(vcpu);
> >
> > + err = emulator_read_emulated(ctxt, rip, buf, sizeof(buf),
> > + &ctxt->exception);
> > + if (err != X86EMUL_CONTINUE)
> > + return err;
> > +
> > kvm_x86_ops->patch_hypercall(vcpu, instruction);
> > + if (!memcmp(instruction, buf, sizeof(instruction)))
> > + /*
> > + * The hypercall instruction is the correct one. Retry
> > + * its execution maybe we got here as a result of an
> > + * event other than #UD which has been resolved in the
> > + * mean time.
> > + */
> > + return X86EMUL_CONTINUE;
> >
> > - return emulator_write_emulated(ctxt, rip, instruction, 3,
> > - &ctxt->exception);
> > + return emulator_write_emulated(ctxt, rip, instruction,
> > + sizeof(instruction), &ctxt->exception);
> > }
