KVM: inject #UD if instruction emulation fails and exit to userspace

Do not kill VM when instruction emulation fails. Inject #UD and report failure to userspace instead. Userspace may choose to reenter guest if vcpu is in userspace (cpl == 3) in which case guest OS will kill offending process and continue running.

Quoted 5 lines · Gleb Natapov

Do not kill VM when instruction emulation fails. Inject #UD and report failure to userspace instead. Userspace may choose to reenter guest if vcpu is in userspace (cpl == 3) in which case guest OS will kill offending process and continue running.

Please use this one instead. Compilation warning fixed.

Signed-off-by Gleb Natapov <[email protected]>

arch/x86/include/asm/kvm_host.h +0 -1

index ed48904..5aa0944 100644
@@ -575,7 +575,6 @@ enum emulation_result {
  #define EMULTYPE_SKIP              (1 << 2)
  int emulate_instruction(struct kvm_vcpu *vcpu,
                        unsigned long cr2, u16 error_code, int emulation_type);
-void kvm_report_emulation_failure(struct kvm_vcpu *cvpu, const char *context);
  void realmode_lgdt(struct kvm_vcpu *vcpu, u16 size, unsigned long address);
  void realmode_lidt(struct kvm_vcpu *vcpu, u16 size, unsigned long address);
 

arch/x86/kvm/mmu.c +0 -0

index 95bee9d..41d2de8 100644
@@ -2788,11 +2788,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code)
                return 1;

       case EMULATE_DO_MMIO:
               ++vcpu->stat.mmio_exits;
-               return 0;
+               /* fall through */
       case EMULATE_FAIL:
-               vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
-               vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
-               vcpu->run->internal.ndata = 0;
               return 0;
       default:
               BUG();

arch/x86/kvm/svm.c +3 -7

index cea916f..58c91f5 100644
@@ -1449,7 +1449,7 @@ static int io_interception(struct vcpu_svm *svm)
        string = (io_info & SVM_IOIO_STR_MASK) != 0;
        in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
        if (string || in)
-               return !(emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO);
+               return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
 
        port = io_info >> 16;
        size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT;

@@ -2300,16 +2300,12 @@ static int iret_interception(struct vcpu_svm *svm)
 
  static int invlpg_interception(struct vcpu_svm *svm)
  {
-       if (emulate_instruction(&svm->vcpu, 0, 0, 0) != EMULATE_DONE)
-               pr_unimpl(&svm->vcpu, "%s: failed\n", __func__);
-       return 1;
+       return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE;
  }
 
  static int emulate_on_interception(struct vcpu_svm *svm)
  {
-       if (emulate_instruction(&svm->vcpu, 0, 0, 0) != EMULATE_DONE)
-               pr_unimpl(&svm->vcpu, "%s: failed\n", __func__);
-       return 1;
+       return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE;
  }
 
  static int cr8_write_interception(struct vcpu_svm *svm)

arch/x86/kvm/vmx.c +4 -24

index 777e00d..e35c479 100644
@@ -3074,7 +3074,7 @@ static int handle_io(struct kvm_vcpu *vcpu)
        ++vcpu->stat.io_exits;
 
        if (string || in)
-               return !(emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO);
+               return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
 
        port = exit_qualification >> 16;
        size = (exit_qualification & 7) + 1;

@@ -3331,22 +3331,7 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu)
 
  static int handle_apic_access(struct kvm_vcpu *vcpu)
  {
-       unsigned long exit_qualification;
-       enum emulation_result er;
-       unsigned long offset;
-
-       exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
-       offset = exit_qualification & 0xffful;
-
-       er = emulate_instruction(vcpu, 0, 0, 0);
-
-       if (er !=  EMULATE_DONE) {
-               printk(KERN_ERR
-                      "Fail to handle apic access vmexit! Offset is 0x%lx\n",
-                      offset);
-               return -ENOEXEC;
-       }
-       return 1;
+       return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
  }
 
  static int handle_task_switch(struct kvm_vcpu *vcpu)

@@ -3558,13 +3543,8 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
                        goto out;
                }
 
-               if (err != EMULATE_DONE) {
-                       vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
-                       vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
-                       vcpu->run->internal.ndata = 0;
-                       ret = 0;
-                       goto out;
-               }
+               if (err != EMULATE_DONE)
+                       return 0;
 
                if (signal_pending(current))
                        goto out;

arch/x86/kvm/x86.c +16 -26

index cd8a606..eb845cf 100644
@@ -3587,24 +3587,6 @@ int emulator_set_dr(int dr, unsigned long value, struct kvm_vcpu *vcpu)
        return __kvm_set_dr(vcpu, dr, value);
  }
 
-void kvm_report_emulation_failure(struct kvm_vcpu *vcpu, const char *context)
-{
-       u8 opcodes[4];
-       unsigned long rip = kvm_rip_read(vcpu);
-       unsigned long rip_linear;
-
-       if (!printk_ratelimit())
-               return;
-
-       rip_linear = rip + get_segment_base(vcpu, VCPU_SREG_CS);
-
-       kvm_read_guest_virt(rip_linear, (void *)opcodes, 4, vcpu, NULL);
-
-       printk(KERN_ERR "emulation failed (%s) rip %lx %02x %02x %02x %02x\n",
-              context, rip, opcodes[0], opcodes[1], opcodes[2], opcodes[3]);
-}
-EXPORT_SYMBOL_GPL(kvm_report_emulation_failure);
-
  static u64 mk_cr_64(u64 curr_cr, u32 new_val)
  {
        return (curr_cr & ~((1ULL << 32) - 1)) | new_val;

@@ -3811,6 +3793,17 @@ static void inject_emulated_exception(struct kvm_vcpu *vcpu)
                kvm_queue_exception(vcpu, ctxt->exception);
  }
 
+static int handle_emulation_failure(struct kvm_vcpu *vcpu)
+{
+       ++vcpu->stat.insn_emulation_fail;
+       trace_kvm_emulate_insn_failed(vcpu);
+       vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+       vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
+       vcpu->run->internal.ndata = 0;
+       kvm_queue_exception(vcpu, UD_VECTOR);
+       return EMULATE_FAIL;
+}
+
  int emulate_instruction(struct kvm_vcpu *vcpu,
                        unsigned long cr2,
                        u16 error_code,

@@ -3879,11 +3872,11 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
 
                ++vcpu->stat.insn_emulation;
                if (r)  {
-                       ++vcpu->stat.insn_emulation_fail;
-                       trace_kvm_emulate_insn_failed(vcpu);
                        if (kvm_mmu_unprotect_page_virt(vcpu, cr2))
                                return EMULATE_DONE;
-                       return EMULATE_FAIL;
+                       if (emulation_type & EMULTYPE_SKIP)
+                               return EMULATE_FAIL;
+                       return handle_emulation_failure(vcpu);
                }
        }
 

@@ -3908,9 +3901,7 @@ restart:
                if (kvm_mmu_unprotect_page_virt(vcpu, cr2))
                        return EMULATE_DONE;
 
-               trace_kvm_emulate_insn_failed(vcpu);
-               kvm_report_emulation_failure(vcpu, "mmio");
-               return EMULATE_FAIL;
+               return handle_emulation_failure(vcpu);
        }
 
        toggle_interruptibility(vcpu, vcpu->arch.emulate_ctxt.interruptibility);

@@ -4746,7 +4737,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
                vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
                r = emulate_instruction(vcpu, 0, 0, EMULTYPE_NO_DECODE);
                srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
-               if (r == EMULATE_DO_MMIO) {
+               if (r != EMULATE_DONE) {
                        r = 0;
                        goto out;
                }
--

                       Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 6 lines · Gleb Natapov

Quoted 5 lines · Gleb Natapov

Do not kill VM when instruction emulation fails. Inject #UD and report failure to userspace instead. Userspace may choose to reenter guest if vcpu is in userspace (cpl == 3) in which case guest OS will kill offending process and continue running.

I am curious to know what'd happen in case the vcpu is in kernel space (cpl == 0). Is that case handled?

Quoted 8 lines · Mohammed Gamal

Quoted 6 lines · Gleb Natapov

Quoted 5 lines · Gleb Natapov

Do not kill VM when instruction emulation fails. Inject #UD and report failure to userspace instead. Userspace may choose to reenter guest if vcpu is in userspace (cpl == 3) in which case guest OS will kill offending process and continue running.

Quoted 3 lines

I am curious to know what'd happen in case the vcpu is in kernel space (cpl == 0). Is that case handled?

Currently no matter where emulation fails VM is stopped and cpu state is printed on stderr. After that patch userspace may choose to continue VM execution after emulation error (#UD will be injected into VM though). The policy is in userspace, but I don't see the point to continue execution after emulation failed in kernel. How kernel can recover from the #UD?

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Do not kill VM when instruction emulation fails. Inject #UD and report failure to userspace instead. Userspace may choose to reenter guest if vcpu is in userspace (cpl == 3) in which case guest OS will kill offending process and continue running.

Signed-off-by Gleb Natapov <[email protected]>

---

Changelog:
v1->v2:
- always inject #UD and report failure to userspace.