linux-kernel - [PATCH] KVM: Addressing a possible race in kvm_vcpu_on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Thu,  9 May 2024 02:01:46 -0700
From: Breno Leitao <leitao@...ian.org>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: rbc@...a.com,
	paulmck@...nel.org,
	kvm@...r.kernel.org (open list:KERNEL VIRTUAL MACHINE (KVM)),
	linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] KVM: Addressing a possible race in kvm_vcpu_on_spin:

There are two workflow paths that access the same address
simultaneously, creating a potential data race in kvm_vcpu_on_spin. This
occurs when one workflow reads kvm->last_boosted_vcpu while another
parallel path writes to it.

KCSAN produces the following output when enabled.

	BUG: KCSAN: data-race in kvm_vcpu_on_spin [kvm] / kvm_vcpu_on_spin [kvm]

	write to 0xffffc90025a92344 of 4 bytes by task 4340 on cpu 16:
	kvm_vcpu_on_spin (arch/x86/kvm/../../../virt/kvm/kvm_main.c:4112) kvm
	handle_pause (arch/x86/kvm/vmx/vmx.c:5929) kvm_intel
	vmx_handle_exit (arch/x86/kvm/vmx/vmx.c:? arch/x86/kvm/vmx/vmx.c:6606) kvm_intel
	vcpu_run (arch/x86/kvm/x86.c:11107 arch/x86/kvm/x86.c:11211) kvm
	kvm_arch_vcpu_ioctl_run (arch/x86/kvm/x86.c:?) kvm
	kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:?) kvm
	__se_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:904 fs/ioctl.c:890)
	__x64_sys_ioctl (fs/ioctl.c:890)
	x64_sys_call (arch/x86/entry/syscall_64.c:33)
	do_syscall_64 (arch/x86/entry/common.c:?)
	entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

	read to 0xffffc90025a92344 of 4 bytes by task 4342 on cpu 4:
	kvm_vcpu_on_spin (arch/x86/kvm/../../../virt/kvm/kvm_main.c:4069) kvm
	handle_pause (arch/x86/kvm/vmx/vmx.c:5929) kvm_intel
	vmx_handle_exit (arch/x86/kvm/vmx/vmx.c:? arch/x86/kvm/vmx/vmx.c:6606) kvm_intel
	vcpu_run (arch/x86/kvm/x86.c:11107 arch/x86/kvm/x86.c:11211) kvm
	kvm_arch_vcpu_ioctl_run (arch/x86/kvm/x86.c:?) kvm
	kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:?) kvm
	__se_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:904 fs/ioctl.c:890)
	__x64_sys_ioctl (fs/ioctl.c:890)
	x64_sys_call (arch/x86/entry/syscall_64.c:33)
	do_syscall_64 (arch/x86/entry/common.c:?)
	entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

	value changed: 0x00000012 -> 0x00000000

Given that both operations occur simultaneously without any locking
mechanisms in place, let's ensure atomicity to prevent possible data
corruption. We'll achieve this by employing READ_ONCE() for the reading
operation and WRITE_ONCE() for the writing operation.

Signed-off-by: Breno Leitao <leitao@...ian.org>
---
 virt/kvm/kvm_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ff0a20565f90..9768307d5e6c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4066,12 +4066,13 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool yield_to_kernel_mode)
 {
 	struct kvm *kvm = me->kvm;
 	struct kvm_vcpu *vcpu;
-	int last_boosted_vcpu = me->kvm->last_boosted_vcpu;
+	int last_boosted_vcpu;
 	unsigned long i;
 	int yielded = 0;
 	int try = 3;
 	int pass;
 
+	last_boosted_vcpu = READ_ONCE(me->kvm->last_boosted_vcpu);
 	kvm_vcpu_set_in_spin_loop(me, true);
 	/*
 	 * We boost the priority of a VCPU that is runnable but not
@@ -4109,7 +4110,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool yield_to_kernel_mode)
 
 			yielded = kvm_vcpu_yield_to(vcpu);
 			if (yielded > 0) {
-				kvm->last_boosted_vcpu = i;
+				WRITE_ONCE(kvm->last_boosted_vcpu, i);
 				break;
 			} else if (yielded < 0) {
 				try--;
-- 
2.43.0