diff mbox

APPLIED: [PATCH 1/1] [CVE-2012-1601] [LUCID] KVM: Ensure all vcpus are consistent with in-kernel irqchip settings

Message ID 4F995557.2000106@canonical.com
State New
Headers show

Commit Message

Tim Gardner April 26, 2012, 2:01 p.m. UTC 
On 04/25/2012 02:15 PM, Tim Gardner wrote:
> 

Given the extra cherry-pick, I'm resubmitting these patches. Pay
particular attention to kvm_arch_vm_ioctl() because my backport is
slightly different then Brad's.

rtg

Comments

Stefan Bader April 26, 2012, 2:30 p.m. UTC | #1 
On 26.04.2012 16:01, Tim Gardner wrote:
> On 04/25/2012 02:15 PM, Tim Gardner wrote:
>>
> 
> Given the extra cherry-pick, I'm resubmitting these patches. Pay
> particular attention to kvm_arch_vm_ioctl() because my backport is
> slightly different then Brad's.
> 
> rtg

@@ -2285,6 +2285,9 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			}
 		} else
 			goto out;
+		r = -EINVAL;
+		if (atomic_read(&kvm->online_vcpus))
+			goto out;
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
 			kfree(kvm->arch.vpic);

That might be slightly too late. The original patch adds the exit before calling
kvm_create_pic. Beside of probably other checks missing and not using the mutex
at all, the version above would have created the pic before bailing out.
diff mbox

Patch

From 271465fe4695618b8a682934b9234ad387f21c36 Mon Sep 17 00:00:00 2001
From: Avi Kivity <avi@redhat.com>
Date: Mon, 5 Mar 2012 14:23:29 +0200
Subject: [PATCH 2/2] KVM: Ensure all vcpus are consistent with in-kernel
 irqchip settings

BugLink: http://bugs.launchpad.net/bugs/971685

CVE-2012-1601

If some vcpus are created before KVM_CREATE_IRQCHIP, then
irqchip_in_kernel() and vcpu->arch.apic will be inconsistent, leading
to potential NULL pointer dereferences.

Fix by:
- ensuring that no vcpus are installed when KVM_CREATE_IRQCHIP is called
- ensuring that a vcpu has an apic if it is installed after KVM_CREATE_IRQCHIP

This is somewhat long winded because vcpu->arch.apic is created without
kvm->lock held.

Based on earlier patch by Michael Ellerman.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Avi Kivity <avi@redhat.com>
(back ported from commit 3e515705a1f46beb1c942bb8043c16f8ac7b1e9e)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 arch/ia64/kvm/kvm-ia64.c |    5 +++++
 arch/x86/kvm/x86.c       |    8 ++++++++
 include/linux/kvm_host.h |    7 +++++++
 virt/kvm/kvm_main.c      |    4 ++++
 4 files changed, 24 insertions(+)

diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 2eb6365..416122b 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1185,6 +1185,11 @@  out:
 
 #define PALE_RESET_ENTRY    0x80000000ffffffb0UL
 
+bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu)
+{
+	return irqchip_in_kernel(vcpu->kcm) == (vcpu->arch.apic != NULL);
+}
+
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct kvm_vcpu *v;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7000a5e..67030b8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2285,6 +2285,9 @@  long kvm_arch_vm_ioctl(struct file *filp,
 			}
 		} else
 			goto out;
+		r = -EINVAL;
+		if (atomic_read(&kvm->online_vcpus))
+			goto out;
 		r = kvm_setup_default_irq_routing(kvm);
 		if (r) {
 			kfree(kvm->arch.vpic);
@@ -5007,6 +5010,11 @@  void kvm_arch_check_processor_compat(void *rtn)
 	kvm_x86_ops->check_processor_compatibility(rtn);
 }
 
+bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu)
+{
+	return irqchip_in_kernel(vcpu->kvm) == (vcpu->arch.apic != NULL);
+}
+
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct page *page;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1296deb..624cf6b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -557,5 +557,12 @@  static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu)
 {
 	return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id;
 }
+
+bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu);
+
+#else
+
+static inline bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu) { return true; }
+
 #endif
 #endif
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 42d7353..a27f14f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1872,6 +1872,10 @@  static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 		goto vcpu_destroy;
 
 	mutex_lock(&kvm->lock);
+	if (!kvm_vcpu_compatible(vcpu)) {
+		r = -EINVAL;
+		goto unlock_vcpu_destroy;
+	}
 	if (atomic_read(&kvm->online_vcpus) == KVM_MAX_VCPUS) {
 		r = -EINVAL;
 		goto unlock_vcpu_destroy;
-- 
1.7.9.5