On 09/08/19 18:00, Adalbert Lazăr wrote: > From: Mihai Donțu <[email protected]> > > 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 > CC: Joerg Roedel <[email protected]> > Signed-off-by: Mihai Donțu <[email protected]> > Signed-off-by: Adalbert Lazăr <[email protected]> > --- > arch/x86/kvm/x86.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > 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); > } > > static int dm_request_for_irq_injection(struct kvm_vcpu *vcpu) > _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
