| Message ID | 4A76EA7B.4080509@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > Actually, VT mandates that vmcalls can only be done from CPL=0. SVM allows hypercalls from CPL>0 unfortunately. Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/2009 05:04 PM, Anthony Liguori wrote: > > 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). > SVM allows hypercalls from CPL>0 unfortunately. Yeah.
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<jan.kiszka@siemens.com> > --- > > 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).
Avi Kivity wrote: > On 08/03/2009 05:04 PM, Anthony Liguori wrote: >> >> 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). Ah, it's used to call SMM peer mode... awesome :-) Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/2009 06:04 PM, Anthony Liguori wrote: >> 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.
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; + } + 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; 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
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 <jan.kiszka@siemens.com> --- arch/x86/kvm/x86.c | 8 ++++++++ include/linux/kvm_para.h | 1 + 2 files changed, 9 insertions(+), 0 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html