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

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

[Qi Zhang]

From: Chengfeng Ye <nicoyip.dev@gmail.com>

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: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Signed-off-by: Qi Zhang <marsy12010123@gmail.com>
---
 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data race on `vcpu->arch.tsc_offset_adjustment` during CPU hotplug can lead to guest timekeeping corruption.
--

--- Patch [1]: [PATCH] KVM: x86: Protect virtualization-enable VM walks with RCU ---
commit ff5afc2b0cb8750aad6e3a35f5b08c17dc92aca8
Author: Chengfeng Ye <nicoyip.dev@gmail.com>

KVM: x86: Protect virtualization-enable VM walks with RCU

This commit protects the vm_list walks in kvm_arch_enable_virtualization_cpu()
with RCU. It publishes and removes VMs using RCU list operations to prevent
use-after-free bugs during CPU hotplug concurrent with VM teardown.

Cc: stable@vger.kernel.org

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index afcac1042947a..fb1c14d7dc4b4 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -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;

[Severity: High]
This is a pre-existing issue, but can this lockless addition race with a
concurrent kvm_arch_vcpu_load() on a running VM?

During CPU hotplug, the hotplug thread iterates over all vCPUs and modifies
tsc_offset_adjustment here locklessly. Concurrently, if the VM is scheduled
on another active CPU, kvm_arch_vcpu_load() could read and clear
vcpu->arch.tsc_offset_adjustment to 0.

Could this data race result in a lost or corrupted TSC adjustment,
potentially warping the guest's timekeeping?

>  			}
>  		}
>  
>  	}
> +	rcu_read_unlock();
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804123243.2213854-1-marsy12010123@gmail.com?part=1