Skip to content

Commit ea61294

Browse files
committed
Revert "KVM: Prevent module exit until all VMs are freed"
Revert KVM's misguided attempt to "fix" a use-after-module-unload bug that was actually due to failure to flush a workqueue, not a lack of module refcounting. Pinning the KVM module until kvm_vm_destroy() doesn't prevent use-after-free due to the module being unloaded, as userspace can invoke delete_module() the instant the last reference to KVM is put, i.e. can cause all KVM code to be unmapped while KVM is actively executing said code. Generally speaking, the many instances of module_put(THIS_MODULE) notwithstanding, outside of a few special paths, a module can never safely put the last reference to itself without creating deadlock, i.e. something external to the module *must* put the last reference. In other words, having VMs grab a reference to the KVM module is futile, pointless, and as evidenced by the now-reverted commit 70375c2 ("Revert "KVM: set owner of cpu and vm file operations""), actively dangerous. This reverts commit 405294f and commit 5f6de5c. Fixes: 405294f ("KVM: Unconditionally get a ref to /dev/kvm module when creating a VM") Fixes: 5f6de5c ("KVM: Prevent module exit until all VMs are freed") Link: https://lore.kernel.org/r/20231018204624.1905300-4-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 087e152 commit ea61294

File tree

1 file changed

+0
-7
lines changed

1 file changed

+0
-7
lines changed

virt/kvm/kvm_main.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,6 @@ EXPORT_SYMBOL_GPL(kvm_debugfs_dir);
115115

116116
static const struct file_operations stat_fops_per_vm;
117117

118-
static struct file_operations kvm_chardev_ops;
119-
120118
static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
121119
unsigned long arg);
122120
#ifdef CONFIG_KVM_COMPAT
@@ -1157,9 +1155,6 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
11571155
if (!kvm)
11581156
return ERR_PTR(-ENOMEM);
11591157

1160-
/* KVM is pinned via open("/dev/kvm"), the fd passed to this ioctl(). */
1161-
__module_get(kvm_chardev_ops.owner);
1162-
11631158
KVM_MMU_LOCK_INIT(kvm);
11641159
mmgrab(current->mm);
11651160
kvm->mm = current->mm;
@@ -1279,7 +1274,6 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
12791274
out_err_no_srcu:
12801275
kvm_arch_free_vm(kvm);
12811276
mmdrop(current->mm);
1282-
module_put(kvm_chardev_ops.owner);
12831277
return ERR_PTR(r);
12841278
}
12851279

@@ -1348,7 +1342,6 @@ static void kvm_destroy_vm(struct kvm *kvm)
13481342
preempt_notifier_dec();
13491343
hardware_disable_all();
13501344
mmdrop(mm);
1351-
module_put(kvm_chardev_ops.owner);
13521345
}
13531346

13541347
void kvm_get_kvm(struct kvm *kvm)

0 commit comments

Comments
 (0)