kvm: fix kvm_ioctl_create_device() reference counting (CVE-2019-6974)
diff mbox series

Message ID 1549562945-5503-1-git-send-email-pbonzini@redhat.com
State New
Headers show
Series
  • kvm: fix kvm_ioctl_create_device() reference counting (CVE-2019-6974)
Related show

Commit Message

Paolo Bonzini Feb. 7, 2019, 6:09 p.m. UTC
From: Jann Horn <jannh@google.com>

kvm_ioctl_create_device() does the following:

1. creates a device that holds a reference to the VM object (with a borrowed
   reference, the VM's refcount has not been bumped yet)
2. initializes the device
3. transfers the reference to the device to the caller's file descriptor table
4. calls kvm_get_kvm() to turn the borrowed reference to the VM into a real
   reference

The ownership transfer in step 3 must not happen before the reference to the VM
becomes a proper, non-borrowed reference, which only happens in step 4.
After step 3, an attacker can close the file descriptor and drop the borrowed
reference, which can cause the refcount of the kvm object to drop to zero.

This means that we need to grab a reference for the device before
anon_inode_getfd(), otherwise the VM can disappear from under us.

Fixes: 852b6d57dc7f ("kvm: add device control API")
Cc: stable@kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/kvm_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Sasha Levin Feb. 11, 2019, 5:26 p.m. UTC | #1
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 852b6d57dc7f kvm: add device control API.

The bot has tested the following trees: v4.20.7, v4.19.20, v4.14.98, v4.9.155, v4.4.173, v3.18.134.

v4.20.7: Build OK!
v4.19.20: Build OK!
v4.14.98: Build OK!
v4.9.155: Build OK!
v4.4.173: Failed to apply! Possible dependencies:
    023e9fddc361 ("KVM: PPC: Move xics_debugfs_init out of create")
    0919e84c0fc1 ("KVM: arm/arm64: vgic-new: Add IRQ sync/flush framework")
    140b086dd197 ("KVM: arm/arm64: vgic-new: Add GICv2 world switch backend")
    2b0cda878965 ("KVM: arm/arm64: vgic-new: Add CTLR, TYPER and IIDR handlers")
    4493b1c4866a ("KVM: arm/arm64: vgic-new: Add MMIO handling framework")
    59529f69f504 ("KVM: arm/arm64: vgic-new: Add GICv3 world switch backend")
    5e6431da8f3a ("KVM: arm/arm64: vgic-new: vgic_init: implement vgic_create")
    64a959d66e47 ("KVM: arm/arm64: vgic-new: Add acccessor to new struct vgic_irq instance")
    81eeb95ddbab ("KVM: arm/arm64: vgic-new: Implement virtual IRQ injection")
    90eee56c5f90 ("KVM: arm/arm64: vgic-new: Implement kvm_vgic_vcpu_pending_irq")
    a28ebea2adc4 ("KVM: Protect device ops->create and list_add with kvm->lock")
    b18b57787f5e ("KVM: arm/arm64: vgic-new: Add data structure definitions")
    c7da6fa43cb1 ("arm/arm64: KVM: Detect vGIC presence at runtime")
    c86c772191d7 ("KVM: arm/arm64: vgic-new: vgic_kvm_device: KVM device ops registration")
    e2c1f9abff83 ("KVM: arm/arm64: vgic-new: vgic_kvm_device: implement kvm_vgic_addr")
    ed9b8cefa916 ("KVM: arm/arm64: vgic-new: Add GICv3 MMIO handling framework")
    fb848db39661 ("KVM: arm/arm64: vgic-new: Add GICv2 MMIO handling framework")
    fca256026bb0 ("KVM: arm/arm64: vgic-new: vgic_kvm_device: KVM_DEV_ARM_VGIC_GRP_NR_IRQS")
    fd59ed3be17e ("KVM: arm/arm64: vgic-new: Add GICv3 CTLR, IIDR, TYPER handlers")

v3.18.134: Failed to apply! Possible dependencies:
    023e9fddc361 ("KVM: PPC: Move xics_debugfs_init out of create")
    05bc8aafe664 ("arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones")
    174178fed338 ("KVM: arm/arm64: add irqfd support")
    1d916229e348 ("arm/arm64: KVM: split GICv2 specific emulation code from vgic.c")
    3caa2d8c3b2d ("arm/arm64: KVM: make the maximum number of vCPUs a per-VM value")
    59892136c40d ("arm/arm64: KVM: pass down user space provided GIC type into vGIC code")
    662d9715840a ("arm/arm64: KVM: Kill CONFIG_KVM_ARM_{VGIC,TIMER}")
    832158125d2e ("arm/arm64: KVM: add vgic.h header file")
    83fe27ea5311 ("rcu: Make SRCU optional by using CONFIG_SRCU")
    96415257a1bd ("arm/arm64: KVM: refactor vgic_handle_mmio() function")
    a28ebea2adc4 ("KVM: Protect device ops->create and list_add with kvm->lock")
    b26e5fdac43c ("arm/arm64: KVM: introduce per-VM ops")
    c7da6fa43cb1 ("arm/arm64: KVM: Detect vGIC presence at runtime")
    d44758c0dfc5 ("KVM: arm/arm64: enable KVM_CAP_IOEVENTFD")
    d97f683d0f4b ("arm/arm64: KVM: refactor MMIO accessors")


How should we proceed with this patch?

--
Thanks,
Sasha

Patch
diff mbox series

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5ecea812cb6a..585845203db8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3000,8 +3000,10 @@  static int kvm_ioctl_create_device(struct kvm *kvm,
 	if (ops->init)
 		ops->init(dev);
 
+	kvm_get_kvm(kvm);
 	ret = anon_inode_getfd(ops->name, &kvm_device_fops, dev, O_RDWR | O_CLOEXEC);
 	if (ret < 0) {
+		kvm_put_kvm(kvm);
 		mutex_lock(&kvm->lock);
 		list_del(&dev->vm_node);
 		mutex_unlock(&kvm->lock);
@@ -3009,7 +3011,6 @@  static int kvm_ioctl_create_device(struct kvm *kvm,
 		return ret;
 	}
 
-	kvm_get_kvm(kvm);
 	cd->fd = ret;
 	return 0;
 }