Skip to content

Commit

Permalink
KVM: set owner of cpu and vm file operations
Browse files Browse the repository at this point in the history
There is a race between a "close of the file descriptors" and module
unload in the kvm module.

You can easily trigger this problem by applying this debug patch:
>--- kvm.orig/virt/kvm/kvm_main.c
>+++ kvm/virt/kvm/kvm_main.c
>@@ -648,10 +648,14 @@ void kvm_free_physmem(struct kvm *kvm)
>                kvm_free_physmem_slot(&kvm->memslots[i], NULL);
> }
>
>+#include <linux/delay.h>
> static void kvm_destroy_vm(struct kvm *kvm)
> {
>        struct mm_struct *mm = kvm->mm;
>
>+       printk("off1\n");
>+       msleep(5000);
>+       printk("off2\n");
>        spin_lock(&kvm_lock);
>        list_del(&kvm->vm_list);
>        spin_unlock(&kvm_lock);

and killing the userspace, followed by an rmmod.

The problem is that kvm_destroy_vm can run while the module count
is 0. That means, you can remove the module while kvm_destroy_vm
is running. But kvm_destroy_vm is part of the module text. This
causes a kerneloops. The race exists without the msleep but is much
harder to trigger.

This patch requires the fix for anon_inodes (anon_inodes: use fops->owner
for module refcount).
With this patch, we can set the owner of all anonymous KVM inodes file
operations. The VFS will then control the KVM module refcount as long as there
is an open file. kvm_destroy_vm will be called by the release function of the
last closed file - before the VFS drops the module refcount.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
  • Loading branch information
borntraeger authored and avikivity committed Dec 31, 2008
1 parent e3a2a0d commit 3d3aab1
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions virt/kvm/kvm_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ static int kvm_vcpu_release(struct inode *inode, struct file *filp)
return 0;
}

static const struct file_operations kvm_vcpu_fops = {
static struct file_operations kvm_vcpu_fops = {
.release = kvm_vcpu_release,
.unlocked_ioctl = kvm_vcpu_ioctl,
.compat_ioctl = kvm_vcpu_ioctl,
Expand Down Expand Up @@ -1892,7 +1892,7 @@ static int kvm_vm_mmap(struct file *file, struct vm_area_struct *vma)
return 0;
}

static const struct file_operations kvm_vm_fops = {
static struct file_operations kvm_vm_fops = {
.release = kvm_vm_release,
.unlocked_ioctl = kvm_vm_ioctl,
.compat_ioctl = kvm_vm_ioctl,
Expand Down Expand Up @@ -2256,6 +2256,8 @@ int kvm_init(void *opaque, unsigned int vcpu_size,
}

kvm_chardev_ops.owner = module;
kvm_vm_fops.owner = module;
kvm_vcpu_fops.owner = module;

r = misc_register(&kvm_dev);
if (r) {
Expand Down

0 comments on commit 3d3aab1

Please sign in to comment.