linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"
@ 2018-03-28  0:48 Wanpeng Li
  2018-03-28  0:48 ` [PATCH v3 1/2] KVM: X86: Introduce handle_ud() Wanpeng Li
  2018-03-28  0:48 ` [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
  0 siblings, 2 replies; 6+ messages in thread
From: Wanpeng Li @ 2018-03-28  0:48 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář,
	Andrew Cooper, Konrad Rzeszutek Wilk, Liran Alon

There is no easy way to force KVM to run an instruction through the emulator 
(by design as that will expose the x86 emulator as a significant attack-surface).
However, we do wish to expose the x86 emulator in case we are testing it
(e.g. via kvm-unit-tests). Therefore, this patch adds a "force emulation prefix"
that is designed to raise #UD which KVM will trap and it's #UD exit-handler will
match "force emulation prefix" to run instruction after prefix by the x86 emulator.
To not expose the x86 emulator by default, we add a module parameter that should 
be off by default.

A simple testcase here:

#include <stdio.h>
#include <string.h>
   
#define HYPERVISOR_INFO 0x40000000
   
#define CPUID(idx, eax, ebx, ecx, edx) \
    asm volatile ( \
    "ud2a; .ascii \"kvm\"; cpuid" \
    :"=b" (*ebx), "=a" (*eax), "=c" (*ecx), "=d" (*edx) \
        :"0"(idx) );  
   
void main()  
{  
	unsigned int eax, ebx, ecx, edx;  
	char string[13];  
   
	CPUID(HYPERVISOR_INFO, &eax, &ebx, &ecx, &edx);  
	*(unsigned int *)(string + 0) = ebx;  
	*(unsigned int *)(string + 4) = ecx;  
	*(unsigned int *)(string + 8) = edx;  
   
	string[12] = 0;  
	if (strncmp(string, "KVMKVMKVM\0\0\0", 12) == 0)
		printf("kvm guest\n");  
	else  
		printf("bare hardware\n");  
}

v2 -> v3:
 * fix compile warning
v1 -> v2:
 * update patch descriptions
 * move handle_ud to x86.c, shared by vmx and svm
 * the parameter is in kvm module 
 * rename parameter to force_emulation_prefix

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Liran Alon <liran.alon@oracle.com>

Wanpeng Li (2):
  KVM: X86: Introduce handle_ud()
  KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"

 arch/x86/kvm/svm.c |  9 +--------
 arch/x86/kvm/vmx.c | 10 ++--------
 arch/x86/kvm/x86.c | 29 +++++++++++++++++++++++++++++
 arch/x86/kvm/x86.h |  2 ++
 4 files changed, 34 insertions(+), 16 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] KVM: X86: Introduce handle_ud()
  2018-03-28  0:48 [PATCH v3 0/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
@ 2018-03-28  0:48 ` Wanpeng Li
  2018-03-28  0:48 ` [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
  1 sibling, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2018-03-28  0:48 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář,
	Andrew Cooper, Konrad Rzeszutek Wilk, Liran Alon

From: Wanpeng Li <wanpengli@tencent.com>

Introduce handle_ud() to handle invalid opcode, this function will be
used by later patches.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Liran Alon <liran.alon@oracle.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Liran Alon <liran.alon@oracle.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kvm/svm.c |  9 +--------
 arch/x86/kvm/vmx.c | 10 ++--------
 arch/x86/kvm/x86.c | 13 +++++++++++++
 arch/x86/kvm/x86.h |  2 ++
 4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index cb46e98..65eb3b9 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2581,14 +2581,7 @@ static int bp_interception(struct vcpu_svm *svm)
 
 static int ud_interception(struct vcpu_svm *svm)
 {
-	int er;
-
-	er = emulate_instruction(&svm->vcpu, EMULTYPE_TRAP_UD);
-	if (er == EMULATE_USER_EXIT)
-		return 0;
-	if (er != EMULATE_DONE)
-		kvm_queue_exception(&svm->vcpu, UD_VECTOR);
-	return 1;
+	return handle_ud(&svm->vcpu);
 }
 
 static int ac_interception(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9bc05f5..63b46db 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6233,14 +6233,8 @@ static int handle_exception(struct kvm_vcpu *vcpu)
 	if (is_nmi(intr_info))
 		return 1;  /* already handled by vmx_vcpu_run() */
 
-	if (is_invalid_opcode(intr_info)) {
-		er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
-		if (er == EMULATE_USER_EXIT)
-			return 0;
-		if (er != EMULATE_DONE)
-			kvm_queue_exception(vcpu, UD_VECTOR);
-		return 1;
-	}
+	if (is_invalid_opcode(intr_info))
+		return handle_ud(vcpu);
 
 	error_code = 0;
 	if (intr_info & INTR_INFO_DELIVER_CODE_MASK)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1583bdc..e3a60ab 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4840,6 +4840,19 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
 }
 EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
 
+int handle_ud(struct kvm_vcpu *vcpu)
+{
+	enum emulation_result er;
+
+	er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
+	if (er == EMULATE_USER_EXIT)
+		return 0;
+	if (er != EMULATE_DONE)
+		kvm_queue_exception(vcpu, UD_VECTOR);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(handle_ud);
+
 static int vcpu_is_mmio_gpa(struct kvm_vcpu *vcpu, unsigned long gva,
 			    gpa_t gpa, bool write)
 {
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index b620cfa..b2f6191 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -219,6 +219,8 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
 	gva_t addr, void *val, unsigned int bytes,
 	struct x86_exception *exception);
 
+int handle_ud(struct kvm_vcpu *vcpu);
+
 void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu);
 u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn);
 bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"
  2018-03-28  0:48 [PATCH v3 0/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
  2018-03-28  0:48 ` [PATCH v3 1/2] KVM: X86: Introduce handle_ud() Wanpeng Li
@ 2018-03-28  0:48 ` Wanpeng Li
  2018-03-29 21:29   ` Radim Krčmář
  1 sibling, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2018-03-28  0:48 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář,
	Andrew Cooper, Konrad Rzeszutek Wilk, Liran Alon

From: Wanpeng Li <wanpengli@tencent.com>

There is no easy way to force KVM to run an instruction through the emulator 
(by design as that will expose the x86 emulator as a significant attack-surface).
However, we do wish to expose the x86 emulator in case we are testing it
(e.g. via kvm-unit-tests). Therefore, this patch adds a "force emulation prefix"
that is designed to raise #UD which KVM will trap and it's #UD exit-handler will
match "force emulation prefix" to run instruction after prefix by the x86 emulator.
To not expose the x86 emulator by default, we add a module parameter that should 
be off by default.

A simple testcase here:

#include <stdio.h>
#include <string.h>
   
#define HYPERVISOR_INFO 0x40000000
   
#define CPUID(idx, eax, ebx, ecx, edx) \
    asm volatile (\
    "ud2a; .ascii \"kvm\"; cpuid" \
    :"=b" (*ebx), "=a" (*eax), "=c" (*ecx), "=d" (*edx) \
        :"0"(idx) );  
   
void main()  
{  
	unsigned int eax, ebx, ecx, edx;  
	char string[13];  
   
	CPUID(HYPERVISOR_INFO, &eax, &ebx, &ecx, &edx);  
	*(unsigned int *)(string + 0) = ebx;  
	*(unsigned int *)(string + 4) = ecx;  
	*(unsigned int *)(string + 8) = edx;  
   
	string[12] = 0;  
	if (strncmp(string, "KVMKVMKVM\0\0\0", 12) == 0)
		printf("kvm guest\n");  
	else  
		printf("bare hardware\n");  
}

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Liran Alon <liran.alon@oracle.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kvm/x86.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e3a60ab..40e2f78 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -146,6 +146,9 @@ bool __read_mostly enable_vmware_backdoor = false;
 module_param(enable_vmware_backdoor, bool, S_IRUGO);
 EXPORT_SYMBOL_GPL(enable_vmware_backdoor);
 
+static bool __read_mostly force_emulation_prefix = false;
+module_param(force_emulation_prefix, bool, S_IRUGO);
+
 #define KVM_NR_SHARED_MSRS 16
 
 struct kvm_shared_msrs_global {
@@ -4843,8 +4846,21 @@ EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
 int handle_ud(struct kvm_vcpu *vcpu)
 {
 	enum emulation_result er;
+	int emulation_type = EMULTYPE_TRAP_UD;
+
+	if (force_emulation_prefix) {
+		char sig[5]; /* ud2; .ascii "kvm" */
+		struct x86_exception e;
+
+		kvm_read_guest_virt(&vcpu->arch.emulate_ctxt,
+				kvm_get_linear_rip(vcpu), sig, sizeof(sig), &e);
+		if (memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
+			emulation_type = 0;
+			kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
+		}
+	}
 
-	er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
+	er = emulate_instruction(vcpu, emulation_type);
 	if (er == EMULATE_USER_EXIT)
 		return 0;
 	if (er != EMULATE_DONE)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"
  2018-03-28  0:48 ` [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
@ 2018-03-29 21:29   ` Radim Krčmář
  2018-04-01 11:12     ` Wanpeng Li
  0 siblings, 1 reply; 6+ messages in thread
From: Radim Krčmář @ 2018-03-29 21:29 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: linux-kernel, kvm, Paolo Bonzini, Andrew Cooper,
	Konrad Rzeszutek Wilk, Liran Alon

2018-03-27 17:48-0700, Wanpeng Li:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> There is no easy way to force KVM to run an instruction through the emulator 
> (by design as that will expose the x86 emulator as a significant attack-surface).
> However, we do wish to expose the x86 emulator in case we are testing it
> (e.g. via kvm-unit-tests). Therefore, this patch adds a "force emulation prefix"
> that is designed to raise #UD which KVM will trap and it's #UD exit-handler will
> match "force emulation prefix" to run instruction after prefix by the x86 emulator.
> To not expose the x86 emulator by default, we add a module parameter that should 
> be off by default.
> 
> A simple testcase here:
> 
> #include <stdio.h>
> #include <string.h>
>    
> #define HYPERVISOR_INFO 0x40000000
>    
> #define CPUID(idx, eax, ebx, ecx, edx) \
>     asm volatile (\
>     "ud2a; .ascii \"kvm\"; cpuid" \
>     :"=b" (*ebx), "=a" (*eax), "=c" (*ecx), "=d" (*edx) \
>         :"0"(idx) );  
>    
> void main()  
> {  
> 	unsigned int eax, ebx, ecx, edx;  
> 	char string[13];  
>    
> 	CPUID(HYPERVISOR_INFO, &eax, &ebx, &ecx, &edx);  
> 	*(unsigned int *)(string + 0) = ebx;  
> 	*(unsigned int *)(string + 4) = ecx;  
> 	*(unsigned int *)(string + 8) = edx;  
>    
> 	string[12] = 0;  
> 	if (strncmp(string, "KVMKVMKVM\0\0\0", 12) == 0)
> 		printf("kvm guest\n");  
> 	else  
> 		printf("bare hardware\n");  
> }
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Liran Alon <liran.alon@oracle.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> @@ -146,6 +146,9 @@ bool __read_mostly enable_vmware_backdoor = false;
>  module_param(enable_vmware_backdoor, bool, S_IRUGO);
>  EXPORT_SYMBOL_GPL(enable_vmware_backdoor);
>  
> +static bool __read_mostly force_emulation_prefix = false;
> +module_param(force_emulation_prefix, bool, S_IRUGO);
> +
>  #define KVM_NR_SHARED_MSRS 16
>  
>  struct kvm_shared_msrs_global {
> @@ -4843,8 +4846,21 @@ EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
>  int handle_ud(struct kvm_vcpu *vcpu)
>  {
>  	enum emulation_result er;
> +	int emulation_type = EMULTYPE_TRAP_UD;
> +
> +	if (force_emulation_prefix) {
> +		char sig[5]; /* ud2; .ascii "kvm" */
> +		struct x86_exception e;
> +
> +		kvm_read_guest_virt(&vcpu->arch.emulate_ctxt,
> +				kvm_get_linear_rip(vcpu), sig, sizeof(sig), &e);
> +		if (memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
> +			emulation_type = 0;
> +			kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
> +		}
> +	}
>  
> -	er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
> +	er = emulate_instruction(vcpu, emulation_type);
>  	if (er == EMULATE_USER_EXIT)
>  		return 0;
>  	if (er != EMULATE_DONE)

The code afterwards is going to inject an #UD if the emulation failed.
I think that preserving the cpu state and forwarding the emulation
failure to userspace would be more useful.  The change would probably be
best as:

  		if (memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
  			kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
  			return emulate_instruction(vcpu, 0) == EMULATE_DONE;
  		}

Looks great otherwise, thanks.

(We want to use this in emulate.c kvm-unit-test that currently fails
 because of a hack that doesn't work anymore.)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"
  2018-03-29 21:29   ` Radim Krčmář
@ 2018-04-01 11:12     ` Wanpeng Li
  0 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2018-04-01 11:12 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: LKML, kvm, Paolo Bonzini, Andrew Cooper, Konrad Rzeszutek Wilk,
	Liran Alon

2018-03-30 5:29 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 2018-03-27 17:48-0700, Wanpeng Li:
>> From: Wanpeng Li <wanpengli@tencent.com>
>>
>> There is no easy way to force KVM to run an instruction through the emulator
>> (by design as that will expose the x86 emulator as a significant attack-surface).
>> However, we do wish to expose the x86 emulator in case we are testing it
>> (e.g. via kvm-unit-tests). Therefore, this patch adds a "force emulation prefix"
>> that is designed to raise #UD which KVM will trap and it's #UD exit-handler will
>> match "force emulation prefix" to run instruction after prefix by the x86 emulator.
>> To not expose the x86 emulator by default, we add a module parameter that should
>> be off by default.
>>
>> A simple testcase here:
>>
>> #include <stdio.h>
>> #include <string.h>
>>
>> #define HYPERVISOR_INFO 0x40000000
>>
>> #define CPUID(idx, eax, ebx, ecx, edx) \
>>     asm volatile (\
>>     "ud2a; .ascii \"kvm\"; cpuid" \
>>     :"=b" (*ebx), "=a" (*eax), "=c" (*ecx), "=d" (*edx) \
>>         :"0"(idx) );
>>
>> void main()
>> {
>>       unsigned int eax, ebx, ecx, edx;
>>       char string[13];
>>
>>       CPUID(HYPERVISOR_INFO, &eax, &ebx, &ecx, &edx);
>>       *(unsigned int *)(string + 0) = ebx;
>>       *(unsigned int *)(string + 4) = ecx;
>>       *(unsigned int *)(string + 8) = edx;
>>
>>       string[12] = 0;
>>       if (strncmp(string, "KVMKVMKVM\0\0\0", 12) == 0)
>>               printf("kvm guest\n");
>>       else
>>               printf("bare hardware\n");
>> }
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Liran Alon <liran.alon@oracle.com>
>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
>> ---
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> @@ -146,6 +146,9 @@ bool __read_mostly enable_vmware_backdoor = false;
>>  module_param(enable_vmware_backdoor, bool, S_IRUGO);
>>  EXPORT_SYMBOL_GPL(enable_vmware_backdoor);
>>
>> +static bool __read_mostly force_emulation_prefix = false;
>> +module_param(force_emulation_prefix, bool, S_IRUGO);
>> +
>>  #define KVM_NR_SHARED_MSRS 16
>>
>>  struct kvm_shared_msrs_global {
>> @@ -4843,8 +4846,21 @@ EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
>>  int handle_ud(struct kvm_vcpu *vcpu)
>>  {
>>       enum emulation_result er;
>> +     int emulation_type = EMULTYPE_TRAP_UD;
>> +
>> +     if (force_emulation_prefix) {
>> +             char sig[5]; /* ud2; .ascii "kvm" */
>> +             struct x86_exception e;
>> +
>> +             kvm_read_guest_virt(&vcpu->arch.emulate_ctxt,
>> +                             kvm_get_linear_rip(vcpu), sig, sizeof(sig), &e);
>> +             if (memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
>> +                     emulation_type = 0;
>> +                     kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
>> +             }
>> +     }
>>
>> -     er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
>> +     er = emulate_instruction(vcpu, emulation_type);
>>       if (er == EMULATE_USER_EXIT)
>>               return 0;
>>       if (er != EMULATE_DONE)
>
> The code afterwards is going to inject an #UD if the emulation failed.
> I think that preserving the cpu state and forwarding the emulation
> failure to userspace would be more useful.  The change would probably be
> best as:
>
>                 if (memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
>                         kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
>                         return emulate_instruction(vcpu, 0) == EMULATE_DONE;
>                 }
>
> Looks great otherwise, thanks.

Do it in v4. :)

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"
@ 2018-03-28  6:55 Liran Alon
  0 siblings, 0 replies; 6+ messages in thread
From: Liran Alon @ 2018-03-28  6:55 UTC (permalink / raw)
  To: kernellwp
  Cc: konrad.wilk, rkrcmar, pbonzini, linux-kernel, andrew.cooper3, kvm


----- kernellwp@gmail.com wrote:

> From: Wanpeng Li <wanpengli@tencent.com>
> 
> There is no easy way to force KVM to run an instruction through the
> emulator 
> (by design as that will expose the x86 emulator as a significant
> attack-surface).
> However, we do wish to expose the x86 emulator in case we are testing
> it
> (e.g. via kvm-unit-tests). Therefore, this patch adds a "force
> emulation prefix"
> that is designed to raise #UD which KVM will trap and it's #UD
> exit-handler will
> match "force emulation prefix" to run instruction after prefix by the
> x86 emulator.
> To not expose the x86 emulator by default, we add a module parameter
> that should 
> be off by default.
> 
> A simple testcase here:
> 
> #include <stdio.h>
> #include <string.h>
>    
> #define HYPERVISOR_INFO 0x40000000
>    
> #define CPUID(idx, eax, ebx, ecx, edx) \
>     asm volatile (\
>     "ud2a; .ascii \"kvm\"; cpuid" \
>     :"=b" (*ebx), "=a" (*eax), "=c" (*ecx), "=d" (*edx) \
>         :"0"(idx) );  
>    
> void main()  
> {  
> 	unsigned int eax, ebx, ecx, edx;  
> 	char string[13];  
>    
> 	CPUID(HYPERVISOR_INFO, &eax, &ebx, &ecx, &edx);  
> 	*(unsigned int *)(string + 0) = ebx;  
> 	*(unsigned int *)(string + 4) = ecx;  
> 	*(unsigned int *)(string + 8) = edx;  
>    
> 	string[12] = 0;  
> 	if (strncmp(string, "KVMKVMKVM\0\0\0", 12) == 0)
> 		printf("kvm guest\n");  
> 	else  
> 		printf("bare hardware\n");  
> }
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Liran Alon <liran.alon@oracle.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  arch/x86/kvm/x86.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e3a60ab..40e2f78 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -146,6 +146,9 @@ bool __read_mostly enable_vmware_backdoor =
> false;
>  module_param(enable_vmware_backdoor, bool, S_IRUGO);
>  EXPORT_SYMBOL_GPL(enable_vmware_backdoor);
>  
> +static bool __read_mostly force_emulation_prefix = false;
> +module_param(force_emulation_prefix, bool, S_IRUGO);
> +
>  #define KVM_NR_SHARED_MSRS 16
>  
>  struct kvm_shared_msrs_global {
> @@ -4843,8 +4846,21 @@
> EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
>  int handle_ud(struct kvm_vcpu *vcpu)
>  {
>  	enum emulation_result er;
> +	int emulation_type = EMULTYPE_TRAP_UD;
> +
> +	if (force_emulation_prefix) {
> +		char sig[5]; /* ud2; .ascii "kvm" */
> +		struct x86_exception e;
> +
> +		kvm_read_guest_virt(&vcpu->arch.emulate_ctxt,
> +				kvm_get_linear_rip(vcpu), sig, sizeof(sig), &e);
> +		if (memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
> +			emulation_type = 0;
> +			kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
> +		}
> +	}
>  
> -	er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
> +	er = emulate_instruction(vcpu, emulation_type);
>  	if (er == EMULATE_USER_EXIT)
>  		return 0;
>  	if (er != EMULATE_DONE)
> -- 
> 2.7.4

Reviewed-by: Liran Alon <liran.alon@oracle.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-04-01 11:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28  0:48 [PATCH v3 0/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
2018-03-28  0:48 ` [PATCH v3 1/2] KVM: X86: Introduce handle_ud() Wanpeng Li
2018-03-28  0:48 ` [PATCH v3 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction" Wanpeng Li
2018-03-29 21:29   ` Radim Krčmář
2018-04-01 11:12     ` Wanpeng Li
2018-03-28  6:55 Liran Alon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).