diff mbox series

[v8,80/81] KVM: introspection: emulate a guest page table walk on SPT violations due to A/D bit updates

Message ID 20200330101308.21702-81-alazar@bitdefender.com (mailing list archive)
State New, archived
Headers show
Series VM introspection | expand

Commit Message

Adalbert Lazăr March 30, 2020, 10:13 a.m. UTC 
From: Mihai Donțu <mdontu@bitdefender.com>

On SPT page faults caused by guest page table walks, use the existing
guest page table walk code to make the necessary adjustments to the A/D
bits and return to guest. This effectively bypasses the x86 emulator
who was making the wrong modifications leading one OS (Windows 8.1 x64)
to triple-fault very early in the boot process with the introspection
enabled.

With introspection disabled, these faults are handled by simply removing
the protection from the affected guest page and returning to guest.

Signed-off-by: Mihai Donțu <mdontu@bitdefender.com>
Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>
---
 arch/x86/include/asm/kvmi_host.h |  2 ++
 arch/x86/kvm/kvmi.c              | 33 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/mmu/mmu.c           | 11 +++++++++--
 include/linux/kvmi_host.h        |  3 +++
 virt/kvm/introspection/kvmi.c    | 26 +++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 2 deletions(-)

Comments

kernel test robot March 31, 2020, 5:32 a.m. UTC | #1 
Hi "Adalbert,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on linus/master v5.6]
[cannot apply to kvm/linux-next vhost/linux-next next-20200330]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Adalbert-Laz-r/VM-introspection/20200330-234749
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-lkp (attached as .config)
compiler: gcc-7 (Debian 7.4.0-6) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: arch/x86/../../virt/kvm/coalesced_mmio.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/../../virt/kvm/eventfd.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/../../virt/kvm/irqchip.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/../../virt/kvm/vfio.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/../../virt/kvm/async_pf.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/x86.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/emulate.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/i8259.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/irq.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/lapic.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/i8254.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/ioapic.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/irq_comm.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/cpuid.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/pmu.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/mtrr.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/hyperv.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/debugfs.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/mmu/mmu.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/mmu/page_track.o: in function `kvmi_update_ad_flags':
>> arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/vmx/vmx.o: in function `kvmi_update_ad_flags':
   arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/vmx/pmu_intel.o: in function `kvmi_update_ad_flags':
   arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/vmx/vmcs12.o: in function `kvmi_update_ad_flags':
   arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/vmx/evmcs.o: in function `kvmi_update_ad_flags':
   arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here
   ld: arch/x86/kvm/vmx/nested.o: in function `kvmi_update_ad_flags':
   arch/x86/include/asm/kvmi_host.h:87: multiple definition of `kvmi_update_ad_flags'; arch/x86/../../virt/kvm/kvm_main.o:arch/x86/include/asm/kvmi_host.h:87: first defined here

vim +87 arch/x86/include/asm/kvmi_host.h

    67	
    68	static inline bool kvmi_monitor_bp_intercept(struct kvm_vcpu *vcpu, u32 dbg)
    69		{ return false; }
    70	static inline bool kvmi_cr_event(struct kvm_vcpu *vcpu, unsigned int cr,
    71					 unsigned long old_value,
    72					 unsigned long *new_value) { return true; }
    73	static inline bool kvmi_cr3_intercepted(struct kvm_vcpu *vcpu) { return false; }
    74	static inline bool kvmi_monitor_cr3w_intercept(struct kvm_vcpu *vcpu,
    75							bool enable) { return false; }
    76	static inline void kvmi_xsetbv_event(struct kvm_vcpu *vcpu) { }
    77	static inline bool kvmi_monitor_desc_intercept(struct kvm_vcpu *vcpu,
    78						       bool enable) { return false; }
    79	static inline bool kvmi_descriptor_event(struct kvm_vcpu *vcpu, u8 descriptor,
    80						 bool write) { return true; }
    81	static inline bool kvmi_msr_event(struct kvm_vcpu *vcpu, struct msr_data *msr)
    82					{ return true; }
    83	static inline bool kvmi_monitor_msrw_intercept(struct kvm_vcpu *vcpu, u32 msr,
    84						       bool enable) { return false; }
    85	static inline bool kvmi_msrw_intercept_originator(struct kvm_vcpu *vcpu)
    86					{ return false; }
  > 87	bool kvmi_update_ad_flags(struct kvm_vcpu *vcpu) { return false; }
    88	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvmi_host.h b/arch/x86/include/asm/kvmi_host.h
index 8d0c3ed3021b..9c9d6a018206 100644
--- a/arch/x86/include/asm/kvmi_host.h
+++ b/arch/x86/include/asm/kvmi_host.h
@@ -61,6 +61,7 @@  bool kvmi_descriptor_event(struct kvm_vcpu *vcpu, u8 descriptor, bool write);
 bool kvmi_msr_event(struct kvm_vcpu *vcpu, struct msr_data *msr);
 bool kvmi_monitor_msrw_intercept(struct kvm_vcpu *vcpu, u32 msr, bool enable);
 bool kvmi_msrw_intercept_originator(struct kvm_vcpu *vcpu);
+bool kvmi_update_ad_flags(struct kvm_vcpu *vcpu);
 
 #else /* CONFIG_KVM_INTROSPECTION */
 
@@ -83,6 +84,7 @@  static inline bool kvmi_monitor_msrw_intercept(struct kvm_vcpu *vcpu, u32 msr,
 					       bool enable) { return false; }
 static inline bool kvmi_msrw_intercept_originator(struct kvm_vcpu *vcpu)
 				{ return false; }
+bool kvmi_update_ad_flags(struct kvm_vcpu *vcpu) { return false; }
 
 #endif /* CONFIG_KVM_INTROSPECTION */
 
diff --git a/arch/x86/kvm/kvmi.c b/arch/x86/kvm/kvmi.c
index d339a879ba0b..f649630b089e 100644
--- a/arch/x86/kvm/kvmi.c
+++ b/arch/x86/kvm/kvmi.c
@@ -1232,3 +1232,36 @@  gpa_t kvmi_arch_cmd_translate_gva(struct kvm_vcpu *vcpu, gva_t gva)
 {
 	return kvm_mmu_gva_to_gpa_system(vcpu, gva, 0, NULL);
 }
+
+bool kvmi_update_ad_flags(struct kvm_vcpu *vcpu)
+{
+	struct kvm_introspection *kvmi;
+	bool ret = false;
+	gva_t gva;
+	gpa_t gpa;
+
+	kvmi = kvmi_get(vcpu->kvm);
+	if (!kvmi)
+		return false;
+
+	gva = kvm_x86_ops->fault_gla(vcpu);
+	if (gva == ~0ull) {
+		kvmi_warn_once(kvmi, "%s: cannot perform translation\n",
+			       __func__);
+		goto out;
+	}
+
+	gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, PFERR_WRITE_MASK, NULL);
+	if (gpa == UNMAPPED_GVA) {
+		struct x86_exception exception = { };
+
+		gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, 0, &exception);
+	}
+
+	ret = (gpa != UNMAPPED_GVA);
+
+out:
+	kvmi_put(vcpu->kvm);
+
+	return ret;
+}
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 35be9f2a2fc7..19a61136e1f1 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5574,8 +5574,15 @@  int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 error_code,
 	 */
 	if (vcpu->arch.mmu->direct_map &&
 	    (error_code & PFERR_NESTED_GUEST_PAGE) == PFERR_NESTED_GUEST_PAGE) {
-		kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(cr2_or_gpa));
-		return 1;
+		gfn_t gfn = gpa_to_gfn(cr2_or_gpa);
+
+		if (kvmi_tracked_gfn(vcpu, gfn)) {
+			if (kvmi_update_ad_flags(vcpu))
+				return 1;
+		} else {
+			kvm_mmu_unprotect_page(vcpu->kvm, gfn);
+			return 1;
+		}
 	}
 
 	/*
diff --git a/include/linux/kvmi_host.h b/include/linux/kvmi_host.h
index 58a30c087d63..a301148c8857 100644
--- a/include/linux/kvmi_host.h
+++ b/include/linux/kvmi_host.h
@@ -97,6 +97,7 @@  bool kvmi_enter_guest(struct kvm_vcpu *vcpu);
 bool kvmi_vcpu_running_singlestep(struct kvm_vcpu *vcpu);
 void kvmi_singlestep_done(struct kvm_vcpu *vcpu);
 void kvmi_singlestep_failed(struct kvm_vcpu *vcpu);
+bool kvmi_tracked_gfn(struct kvm_vcpu *vcpu, gfn_t gfn);
 
 #else
 
@@ -117,6 +118,8 @@  static inline bool kvmi_vcpu_running_singlestep(struct kvm_vcpu *vcpu)
 			{ return false; }
 static inline void kvmi_singlestep_done(struct kvm_vcpu *vcpu) { }
 static inline void kvmi_singlestep_failed(struct kvm_vcpu *vcpu) { }
+static inline bool kvmi_tracked_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
+			{ return false; }
 
 #endif /* CONFIG_KVM_INTROSPECTION */
 
diff --git a/virt/kvm/introspection/kvmi.c b/virt/kvm/introspection/kvmi.c
index 8a73fac287b4..9176ae147d2a 100644
--- a/virt/kvm/introspection/kvmi.c
+++ b/virt/kvm/introspection/kvmi.c
@@ -1381,3 +1381,29 @@  void kvmi_singlestep_failed(struct kvm_vcpu *vcpu)
 	kvmi_handle_singlestep_exit(vcpu, false);
 }
 EXPORT_SYMBOL(kvmi_singlestep_failed);
+
+static bool __kvmi_tracked_gfn(struct kvm_introspection *kvmi, gfn_t gfn)
+{
+	u8 ignored_access;
+
+	if (kvmi_get_gfn_access(kvmi, gfn, &ignored_access))
+		return false;
+
+	return true;
+}
+
+bool kvmi_tracked_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
+{
+	struct kvm_introspection *kvmi;
+	bool ret;
+
+	kvmi = kvmi_get(vcpu->kvm);
+	if (!kvmi)
+		return false;
+
+	ret = __kvmi_tracked_gfn(kvmi, gfn);
+
+	kvmi_put(vcpu->kvm);
+
+	return ret;
+}