Skip to contentSkip to conversation

KVM: x86: Disallow hypercalls for guest callers in rings > 0

PATCH Superseded Jan Kiszka 3 participants 8 messages Aug 3, 2009 – Aug 4, 2009

Subsystems (4)
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 KVM: x86: Disallow hypercalls for guest callers in rings > 0

So far unprivileged guest callers running in ring 3 can issue, e.g., MMU hypercalls. Normally, such callers cannot provide any hand-crafted MMU command structure as it has to be passed by its physical address, but they can still crash the guest kernel by passing random addresses.

To close the hole, this patch considers hypercalls valid only if issued from guest ring 0. This may still be relaxed on a per-hypercall base in the future once required.

---

 arch/x86/kvm/x86.c       |    8 ++++++++
 include/linux/kvm_para.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +8 -0
index 2539e9a..966d309 100644
@@ -3190,6 +3190,7 @@ static inline gpa_t hc_gpa(struct kvm_vcpu *vcpu, unsigned long a0,
 int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 {
 	unsigned long nr, a0, a1, a2, a3, ret;
+	struct kvm_segment cs;
 	int r = 1;
 
 	nr = kvm_register_read(vcpu, VCPU_REGS_RAX);
@@ -3208,6 +3209,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 		a3 &= 0xFFFFFFFF;
 	}
 
+	kvm_get_segment(vcpu, &cs, VCPU_SREG_CS);
+	if (cs.dpl != 0) {
+		ret = -KVM_EPERM;
+		goto out;
+	}
+
 	switch (nr) {
 	case KVM_HC_VAPIC_POLL_IRQ:
 		ret = 0;
@@ -3219,6 +3226,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 		ret = -KVM_ENOSYS;
 		break;
 	}
+out:
 	kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
 	++vcpu->stat.hypercalls;
 	return r;
include/linux/kvm_para.h +1 -0
index 3ddce03..d731092 100644
@@ -13,6 +13,7 @@
 #define KVM_ENOSYS		1000
 #define KVM_EFAULT		EFAULT
 #define KVM_E2BIG		E2BIG
+#define KVM_EPERM		EPERM
 
 #define KVM_HC_VAPIC_POLL_IRQ		1
 #define KVM_HC_MMU_OP			2
#1 lore raw
Quoted 9 lines · Jan Kiszka

So far unprivileged guest callers running in ring 3 can issue, e.g., MMU hypercalls. Normally, such callers cannot provide any hand-crafted MMU command structure as it has to be passed by its physical address, but they can still crash the guest kernel by passing random addresses.

To close the hole, this patch considers hypercalls valid only if issued from guest ring 0. This may still be relaxed on a per-hypercall base in the future once required.

Actually, VT mandates that vmcalls can only be done from CPL=0.

SVM allows hypercalls from CPL>0 unfortunately.

Regards,

Anthony Liguori

#2 lore raw
Quoted 3 lines · Anthony Liguori

Actually, VT mandates that vmcalls can only be done from CPL=0.

That's exactly how I misremembered it. However the docs say

IF not in VMX operation

    THEN #UD;
ELSIF in VMX non-root operation
    THEN VM exit;
ELSIF (RFLAGS.VM = 1) OR (IA32_EFER.LMA = 1 and CS.L = 0)
    THEN #UD;
ELSIF CPL > 0
    THEN #GP(0);

So CPL > 0 is only enforced on VMCALL from the hypervisor, not the guest (tip: don't ask what VMCALL in the hypervisor means).

Quoted 1 line

SVM allows hypercalls from CPL>0 unfortunately.

Yeah.

Signature · 1 line
error compiling committee.c: too many arguments to function
#3 lore raw
Quoted 16 lines · Avi Kivity
Quoted 3 lines · Anthony Liguori

Actually, VT mandates that vmcalls can only be done from CPL=0.

That's exactly how I misremembered it. However the docs say

IF not in VMX operation

    THEN #UD;
ELSIF in VMX non-root operation
    THEN VM exit;
ELSIF (RFLAGS.VM = 1) OR (IA32_EFER.LMA = 1 and CS.L = 0)
    THEN #UD;
ELSIF CPL > 0
    THEN #GP(0);
Quoted 1 line

So CPL > 0 is only enforced on VMCALL from the hypervisor, not the guest (tip: don't ask what VMCALL in the hypervisor means).

Ah, it's used to call SMM peer mode... awesome :-)

Regards,

Anthony Liguori

#4 lore raw
Quoted 4 lines · Anthony Liguori
Quoted 1 line

So CPL > 0 is only enforced on VMCALL from the hypervisor, not the guest (tip: don't ask what VMCALL in the hypervisor means).

Ah, it's used to call SMM peer mode... awesome :-)

Now you'll never be able to sleep at night. Well I did try to warn you.

Signature · 1 line
error compiling committee.c: too many arguments to function
#5 lore raw
In reply to Jan Kiszka
Quoted 29 lines · Jan Kiszka

So far unprivileged guest callers running in ring 3 can issue, e.g., MMU hypercalls. Normally, such callers cannot provide any hand-crafted MMU command structure as it has to be passed by its physical address, but they can still crash the guest kernel by passing random addresses.

To close the hole, this patch considers hypercalls valid only if issued from guest ring 0. This may still be relaxed on a per-hypercall base in the future once required.

---

  arch/x86/kvm/x86.c       |    8 ++++++++
  include/linux/kvm_para.h |    1 +
  2 files changed, 9 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +1 -0
index 2539e9a..966d309 100644
@@ -3190,6 +3190,7 @@ static inline gpa_t hc_gpa(struct kvm_vcpu *vcpu, unsigned long a0,
  int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
  {
  	unsigned long nr, a0, a1, a2, a3, ret;
+	struct kvm_segment cs;
  	int r = 1;
 
  	nr = kvm_register_read(vcpu, VCPU_REGS_RAX);

arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 3218

Quoted 10 lines
arch/x86/kvm/x86.c +6 -0
@@ -3208,6 +3209,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
  		a3&= 0xFFFFFFFF;
  	}
 
+	kvm_get_segment(vcpu,&cs, VCPU_SREG_CS);
+	if (cs.dpl != 0) {
+		ret = -KVM_EPERM;
+		goto out;
+	}
+
 

I think kvm_x86_ops->get_cpl() is more accurate (and we can optimize it to avoid a ton of vmcs_read()s).

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#6 lore raw

PATCH v2 1/1 KVM: x86: Disallow hypercalls for guest callers in rings > 0

Quoted 34 lines · Avi Kivity
Quoted 33 lines · Jan Kiszka

So far unprivileged guest callers running in ring 3 can issue, e.g., MMU hypercalls. Normally, such callers cannot provide any hand-crafted MMU command structure as it has to be passed by its physical address, but they can still crash the guest kernel by passing random addresses.

To close the hole, this patch considers hypercalls valid only if issued from guest ring 0. This may still be relaxed on a per-hypercall base in the future once required.

---

  arch/x86/kvm/x86.c       |    8 ++++++++
  include/linux/kvm_para.h |    1 +
  2 files changed, 9 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +1 -0
index 2539e9a..966d309 100644
@@ -3190,6 +3190,7 @@ static inline gpa_t hc_gpa(struct kvm_vcpu
 *vcpu, unsigned long a0,
  int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
  {
      unsigned long nr, a0, a1, a2, a3, ret;
+    struct kvm_segment cs;
      int r = 1;
 
      nr = kvm_register_read(vcpu, VCPU_REGS_RAX);
arch/x86/kvm/x86.c +0 -0
@@ -3208,6 +3209,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
          a3&= 0xFFFFFFFF;
      }
 
Quoted 11 lines

arch/x86/kvm/x86.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 3218

Quoted 7 lines
arch/x86/kvm/x86.c +6 -0
+    kvm_get_segment(vcpu,&cs, VCPU_SREG_CS);
+    if (cs.dpl != 0) {
+        ret = -KVM_EPERM;
+        goto out;
+    }
+
 

I think kvm_x86_ops->get_cpl() is more accurate (and we can optimize it to avoid a ton of vmcs_read()s).

Yes, that's much nicer.

--------->

So far unprivileged guest callers running in ring 3 can issue, e.g., MMU hypercalls. Normally, such callers cannot provide any hand-crafted MMU command structure as it has to be passed by its physical address, but they can still crash the guest kernel by passing random addresses.

To close the hole, this patch considers hypercalls valid only if issued from guest ring 0. This may still be relaxed on a per-hypercall base in the future once required.

Changes v1 -> v2:
- use kvm_x86_ops->get_cpl() in favor of kvm_get_segment()

---

 arch/x86/kvm/x86.c       |    6 ++++++
 include/linux/kvm_para.h |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +6 -0
index 2539e9a..1c17105 100644
@@ -3208,6 +3208,11 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 		a3 &= 0xFFFFFFFF;
 	}
 
+	if (kvm_x86_ops->get_cpl(vcpu) != 0) {
+		ret = -KVM_EPERM;
+		goto out;
+	}
+
 	switch (nr) {
 	case KVM_HC_VAPIC_POLL_IRQ:
 		ret = 0;
@@ -3219,6 +3224,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 		ret = -KVM_ENOSYS;
 		break;
 	}
+out:
 	kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
 	++vcpu->stat.hypercalls;
 	return r;
include/linux/kvm_para.h +1 -0
index 3ddce03..d731092 100644
@@ -13,6 +13,7 @@
 #define KVM_ENOSYS		1000
 #define KVM_EFAULT		EFAULT
 #define KVM_E2BIG		E2BIG
+#define KVM_EPERM		EPERM
 
 #define KVM_HC_VAPIC_POLL_IRQ		1
 #define KVM_HC_MMU_OP			2
Applied
#7 lore raw
Quoted 10 lines · Jan Kiszka

Yes, that's much nicer.

--------->

So far unprivileged guest callers running in ring 3 can issue, e.g., MMU hypercalls. Normally, such callers cannot provide any hand-crafted MMU command structure as it has to be passed by its physical address, but they can still crash the guest kernel by passing random addresses.

Quoted 8 lines

To close the hole, this patch considers hypercalls valid only if issued from guest ring 0. This may still be relaxed on a per-hypercall base in the future once required.

Changes v1 -> v2:
- use kvm_x86_ops->get_cpl() in favor of kvm_get_segment()

Applied, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function