Skip to content

Commit

Permalink
KVM: Prevent module exit until all VMs are freed
Browse files Browse the repository at this point in the history
Tie the lifetime the KVM module to the lifetime of each VM via
kvm.users_count. This way anything that grabs a reference to the VM via
kvm_get_kvm() cannot accidentally outlive the KVM module.

Prior to this commit, the lifetime of the KVM module was tied to the
lifetime of /dev/kvm file descriptors, VM file descriptors, and vCPU
file descriptors by their respective file_operations "owner" field.
This approach is insufficient because references grabbed via
kvm_get_kvm() do not prevent closing any of the aforementioned file
descriptors.

This fixes a long standing theoretical bug in KVM that at least affects
async page faults. kvm_setup_async_pf() grabs a reference via
kvm_get_kvm(), and drops it in an asynchronous work callback. Nothing
prevents the VM file descriptor from being closed and the KVM module
from being unloaded before this callback runs.

Fixes: af585b9 ("KVM: Halt vcpu if page it tries to access is swapped out")
Fixes: 3d3aab1 ("KVM: set owner of cpu and vm file operations")
Cc: stable@vger.kernel.org
Suggested-by: Ben Gardon <bgardon@google.com>
[ Based on a patch from Ben implemented for Google's kernel. ]
Signed-off-by: David Matlack <dmatlack@google.com>
Message-Id: <20220303183328.1499189-2-dmatlack@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
dmatlack authored and bonzini committed Mar 29, 2022
1 parent c9b8fec commit 5f6de5c
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions virt/kvm/kvm_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ EXPORT_SYMBOL_GPL(kvm_debugfs_dir);

static const struct file_operations stat_fops_per_vm;

static struct file_operations kvm_chardev_ops;

static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
unsigned long arg);
#ifdef CONFIG_KVM_COMPAT
Expand Down Expand Up @@ -1131,6 +1133,16 @@ static struct kvm *kvm_create_vm(unsigned long type)
preempt_notifier_inc();
kvm_init_pm_notifier(kvm);

/*
* When the fd passed to this ioctl() is opened it pins the module,
* but try_module_get() also prevents getting a reference if the module
* is in MODULE_STATE_GOING (e.g. if someone ran "rmmod --wait").
*/
if (!try_module_get(kvm_chardev_ops.owner)) {
r = -ENODEV;
goto out_err;
}

return kvm;

out_err:
Expand Down Expand Up @@ -1220,6 +1232,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
preempt_notifier_dec();
hardware_disable_all();
mmdrop(mm);
module_put(kvm_chardev_ops.owner);
}

void kvm_get_kvm(struct kvm *kvm)
Expand Down

0 comments on commit 5f6de5c

Please sign in to comment.