[PATCH] KVM: x86: Protect virtualization-enable VM walks with RCU

Qi Zhang <[email protected]>
Newsgroups gmane.comp.emulators.kvm.devel,gmane.linux.kernel,gmane.linux.kernel.stable
Message-ID <[email protected]>
From: Chengfeng Ye <[email protected]>

Protect the vm_list walks in kvm_arch_enable_virtualization_cpu() with
RCU, publish and remove VMs with RCU list operations, and wait for
pre-existing readers before tearing down a VM.

The change that introduced kvm_usage_lock replaced kvm_lock with a
dedicated usage-count mutex when enabling virtualization.  Because
vm_list mutators do not take the usage-count mutex, this left the x86
walks without lifetime protection.

During CPU online, the hotplug thread can load a VM from vm_list.  A
concurrent close of the last VM file can then remove that VM and destroy
its vCPUs before the hotplug thread dereferences the stale VM or vCPU.
Reacquiring kvm_lock from the CPU-hotplug path would restore the lock cycle
that the same change fixed.

KASAN reported:

  BUG: KASAN: wild-memory-access in kvm_arch_enable_virtualization_cpu+0x25f/0x870
  Read of size 4 at addr dead0000000000f0 by task cpuhp/3/31
  Call Trace:
   kvm_arch_enable_virtualization_cpu+0x25f/0x870
   kvm_online_cpu+0x1a/0x50
   cpuhp_invoke_callback+0x291/0xfd0
   cpuhp_thread_fun+0x256/0x4a0
   smpboot_thread_fn+0x287/0x6c0

Fixes: 44d174596260 ("KVM: Use dedicated mutex to protect kvm_usage_count to avoid deadlock")
Cc: [email protected]
Signed-off-by: Chengfeng Ye <[email protected]>
Signed-off-by: Qi Zhang <[email protected]>
---
 arch/x86/kvm/x86.c  | 6 ++++--
 virt/kvm/kvm_main.c | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 47cb9eba113b..811014d04d40 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13140,7 +13140,8 @@ int kvm_arch_enable_virtualization_cpu(void)
 
 	local_tsc = rdtsc();
 	stable = !kvm_check_tsc_unstable();
-	list_for_each_entry(kvm, &vm_list, vm_list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(kvm, &vm_list, vm_list) {
 		kvm_for_each_vcpu(i, vcpu, kvm) {
 			if (!stable && vcpu->cpu == smp_processor_id())
 				kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
@@ -13192,7 +13193,7 @@ int kvm_arch_enable_virtualization_cpu(void)
 	 */
 	if (backwards_tsc) {
 		u64 delta_cyc = max_tsc - local_tsc;
-		list_for_each_entry(kvm, &vm_list, vm_list) {
+		list_for_each_entry_rcu(kvm, &vm_list, vm_list) {
 			kvm->arch.backwards_tsc_observed = true;
 			kvm_for_each_vcpu(i, vcpu, kvm) {
 				vcpu->arch.tsc_offset_adjustment += delta_cyc;
@@ -13211,6 +13212,7 @@ int kvm_arch_enable_virtualization_cpu(void)
 		}
 
 	}
+	rcu_read_unlock();
 	return 0;
 }
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 45e784462ec6..92df17710881 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1198,7 +1198,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 		goto out_err_no_debugfs;
 
 	mutex_lock(&kvm_lock);
-	list_add(&kvm->vm_list, &vm_list);
+	list_add_rcu(&kvm->vm_list, &vm_list);
 	mutex_unlock(&kvm_lock);
 
 	preempt_notifier_inc();
@@ -1260,8 +1260,9 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
 	kvm_destroy_vm_debugfs(kvm);
 	mutex_lock(&kvm_lock);
-	list_del(&kvm->vm_list);
+	list_del_rcu(&kvm->vm_list);
 	mutex_unlock(&kvm_lock);
+	synchronize_rcu();
 	kvm_arch_pre_destroy_vm(kvm);
 
 	kvm_free_irq_routing(kvm);
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.