2018-03-27 09:09:16

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 0/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"

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");
}

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 <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Andrew Cooper <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Liran Alon <[email protected]>

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



2018-03-27 09:09:50

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"

From: Wanpeng Li <[email protected]>

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 <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Andrew Cooper <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Liran Alon <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
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);

+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


2018-03-27 10:48:40

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 1/2] KVM: X86: Introduce handle_ud()

From: Wanpeng Li <[email protected]>

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

Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
Reviewed-by: Liran Alon <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Andrew Cooper <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Liran Alon <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
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


2018-03-27 21:18:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"

Hi Wanpeng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kvm/linux-next]
[also build test WARNING on next-20180327]
[cannot apply to v4.16-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Wanpeng-Li/KVM-X86-Add-Force-Emulation-Prefix-for-emulate-the-next-instruction/20180328-025804
base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> arch/x86/kvm/x86.c:149:20: sparse: symbol 'force_emulation_prefix' was not declared. Should it be static?
arch/x86/kvm/x86.c:2196:38: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] <asn:1>*<noident> @@ got d const [noderef] <asn:1>*<noident> @@
arch/x86/kvm/x86.c:2196:38: expected void const [noderef] <asn:1>*<noident>
arch/x86/kvm/x86.c:2196:38: got unsigned char [usertype] *
arch/x86/kvm/x86.c:7911:5: sparse: symbol 'kvm_valid_sregs' was not declared. Should it be static?
arch/x86/kvm/x86.c:8786:16: sparse: incompatible types in comparison expression (different address spaces)
arch/x86/include/asm/paravirt.h:783:9: sparse: context imbalance in 'vcpu_enter_guest' - unexpected unlock

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

2018-03-27 21:22:10

by kernel test robot

[permalink] [raw]
Subject: [RFC PATCH] KVM: X86: force_emulation_prefix can be static


Fixes: 9e028a15a849 ("KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"")
Signed-off-by: Fengguang Wu <[email protected]>
---
x86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 17bb968..44f1e72 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -146,7 +146,7 @@ bool __read_mostly enable_vmware_backdoor = false;
module_param(enable_vmware_backdoor, bool, S_IRUGO);
EXPORT_SYMBOL_GPL(enable_vmware_backdoor);

-bool __read_mostly force_emulation_prefix = false;
+static bool __read_mostly force_emulation_prefix = false;
module_param(force_emulation_prefix, bool, S_IRUGO);

#define KVM_NR_SHARED_MSRS 16

2018-03-28 00:59:22

by Wanpeng Li

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] KVM: X86: Add Force Emulation Prefix for "emulate the next instruction"

2018-03-28 5:15 GMT+08:00 kbuild test robot <[email protected]>:
> Hi Wanpeng,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on kvm/linux-next]
> [also build test WARNING on next-20180327]
> [cannot apply to v4.16-rc7]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Wanpeng-Li/KVM-X86-Add-Force-Emulation-Prefix-for-emulate-the-next-instruction/20180328-025804
> base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> arch/x86/kvm/x86.c:149:20: sparse: symbol 'force_emulation_prefix' was not declared. Should it be static?
> arch/x86/kvm/x86.c:2196:38: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] <asn:1>*<noident> @@ got d const [noderef] <asn:1>*<noident> @@
> arch/x86/kvm/x86.c:2196:38: expected void const [noderef] <asn:1>*<noident>
> arch/x86/kvm/x86.c:2196:38: got unsigned char [usertype] *
> arch/x86/kvm/x86.c:7911:5: sparse: symbol 'kvm_valid_sregs' was not declared. Should it be static?
> arch/x86/kvm/x86.c:8786:16: sparse: incompatible types in comparison expression (different address spaces)
> arch/x86/include/asm/paravirt.h:783:9: sparse: context imbalance in 'vcpu_enter_guest' - unexpected unlock
>
> Please review and possibly fold the followup patch.

Thanks for the report, I will fix it in the next version.

Regards,
Wanpeng Li