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

Commit Message

Jan Kiszka Aug. 3, 2009, 4:43 p.m. UTC

Avi Kivity wrote:
> On 08/03/2009 04:47 PM, Jan Kiszka wrote:
>> 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.
>>
>> Signed-off-by: Jan Kiszka<[e-mail]>
>> ---
>>
>>   arch/x86/kvm/x86.c       |    8 ++++++++
>>   include/linux/kvm_para.h |    1 +
>>   2 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 2539e9a..966d309 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -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;
>> +    }
>> +
>>    
> 
> 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()

Signed-off-by: Jan Kiszka <[e-mail]>
---

 arch/x86/kvm/x86.c       |    6 ++++++
 include/linux/kvm_para.h |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)

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

Comments

Avi Kivity Aug. 4, 2009, noon UTC | #1

On 08/03/2009 07:43 PM, Jan Kiszka wrote:
>
> 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()
>
>    

Applied, thanks.

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2539e9a..1c17105 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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;
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index 3ddce03..d731092 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -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