Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv
@ 2026-06-12 23:06 Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 1/8] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

Fix a bug found by syzkaller (on a Google-internal kernel) where KVM consumes
a vCPU's HyperV structure before it's fully initialized, by concurrently
triggering PV TLB flushes (queues flushes into a vCPU's FIFO without holding
the vCPU's mutex) on a vCPU that is in the process of activating HyperV.

Harden against similar bugs by asserting the vcpu->mutex is held when using
the "normal" to_hv_vcpu(), same as we did for get_vmcs12() and
get_shadow_vmcs12() (also in response to cross-task races).  To avoid false
positives when creating a vCPU, initialize vcpu_idx to -1, and treat the vCPU
as unreachable (other than the caller, obviously) if its index is -1.

v2:
 - Init vcpu->vcpu_idx to -1, use that as a canary to detect the vCPU is
   unreachable, and allow accessing Hyper-V state if the vCPU is otherwise
   unreachable. [syzbot]

v1: https://lore.kernel.org/all/20260423140833.439512-1-seanjc@google.com

Sean Christopherson (8):
  KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller
  KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in
    kvm_hv_get_tlb_flush_fifo()
  KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on
    cross-vCPU accesses
  KVM: Initialize a vCPU's index to '-1' while it's being created
  KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper
  KVM: x86: Treat a vCPU as unreachable if its index is invalid
  KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()
  KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active
    accesses

 arch/x86/kvm/hyperv.c     | 64 +++++++++++++++++++++------------------
 arch/x86/kvm/hyperv.h     | 25 +++++++++++++--
 arch/x86/kvm/vmx/nested.h |  6 ++--
 include/linux/kvm_host.h  |  7 +++++
 virt/kvm/kvm_main.c       |  2 ++
 5 files changed, 67 insertions(+), 37 deletions(-)


base-commit: c1f7303302927f9cbf4efedf70f0512cde168c65
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/8] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 2/8] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo() Sean Christopherson
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

When handling Hyper-V PV TLB flushes, retrieve the to-be-used FIFO in
hv_tlb_flush_enqueue() instead of having the caller pass in the FIFO.  This
will make it easier to fix a cross-vCPU race where KVM can access a vCPU's
FIFO before it's fully initialized.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index fd4eb1e561f7..a894e3d2e594 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1935,16 +1935,18 @@ static int kvm_hv_get_tlb_flush_entries(struct kvm *kvm, struct kvm_hv_hcall *hc
 	return kvm_hv_get_hc_data(kvm, hc, hc->rep_cnt, hc->rep_cnt, entries);
 }
 
-static void hv_tlb_flush_enqueue(struct kvm_vcpu *vcpu,
-				 struct kvm_vcpu_hv_tlb_flush_fifo *tlb_flush_fifo,
-				 u64 *entries, int count)
+static void hv_tlb_flush_enqueue(struct kvm_vcpu *vcpu, u64 *entries, int count,
+				 bool is_guest_mode)
 {
+	struct kvm_vcpu_hv_tlb_flush_fifo *tlb_flush_fifo;
 	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
 	u64 flush_all_entry = KVM_HV_TLB_FLUSHALL_ENTRY;
 
 	if (!hv_vcpu)
 		return;
 
+	tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(vcpu, is_guest_mode);
+
 	spin_lock(&tlb_flush_fifo->write_lock);
 
 	/*
@@ -2017,7 +2019,6 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 	struct kvm *kvm = vcpu->kvm;
 	struct hv_tlb_flush_ex flush_ex;
 	struct hv_tlb_flush flush;
-	struct kvm_vcpu_hv_tlb_flush_fifo *tlb_flush_fifo;
 	/*
 	 * Normally, there can be no more than 'KVM_HV_TLB_FLUSH_FIFO_SIZE'
 	 * entries on the TLB flush fifo. The last entry, however, needs to be
@@ -2145,11 +2146,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 	 * analyze it here, flush TLB regardless of the specified address space.
 	 */
 	if (all_cpus && !is_guest_mode(vcpu)) {
-		kvm_for_each_vcpu(i, v, kvm) {
-			tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(v, false);
-			hv_tlb_flush_enqueue(v, tlb_flush_fifo,
-					     tlb_flush_entries, hc->rep_cnt);
-		}
+		kvm_for_each_vcpu(i, v, kvm)
+			hv_tlb_flush_enqueue(v, tlb_flush_entries, hc->rep_cnt, false);
 
 		kvm_make_all_cpus_request(kvm, KVM_REQ_HV_TLB_FLUSH);
 	} else if (!is_guest_mode(vcpu)) {
@@ -2159,9 +2157,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 			v = kvm_get_vcpu(kvm, i);
 			if (!v)
 				continue;
-			tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(v, false);
-			hv_tlb_flush_enqueue(v, tlb_flush_fifo,
-					     tlb_flush_entries, hc->rep_cnt);
+			hv_tlb_flush_enqueue(v, tlb_flush_entries, hc->rep_cnt, false);
 		}
 
 		kvm_make_vcpus_request_mask(kvm, KVM_REQ_HV_TLB_FLUSH, vcpu_mask);
@@ -2192,9 +2188,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 				continue;
 
 			__set_bit(i, vcpu_mask);
-			tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(v, true);
-			hv_tlb_flush_enqueue(v, tlb_flush_fifo,
-					     tlb_flush_entries, hc->rep_cnt);
+			hv_tlb_flush_enqueue(v, tlb_flush_entries, hc->rep_cnt, true);
 		}
 
 		kvm_make_vcpus_request_mask(kvm, KVM_REQ_HV_TLB_FLUSH, vcpu_mask);
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/8] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo()
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 1/8] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

Check for a NULL Hyper-V object in kvm_hv_get_tlb_flush_fifo() instead of
relying on the caller to do so.  This will allow fixing a cross-vCPU race
where KVM can access a vCPU's FIFO before it's fully initialized, without
having to jump through too many cognitive hoops to reason about the
correctness of the logic.

Ignoring changes in ordering that only affect the aforementioned race, no
functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 11 +++++------
 arch/x86/kvm/hyperv.h |  7 ++++++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index a894e3d2e594..ecd344b91739 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1939,13 +1939,11 @@ static void hv_tlb_flush_enqueue(struct kvm_vcpu *vcpu, u64 *entries, int count,
 				 bool is_guest_mode)
 {
 	struct kvm_vcpu_hv_tlb_flush_fifo *tlb_flush_fifo;
-	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
 	u64 flush_all_entry = KVM_HV_TLB_FLUSHALL_ENTRY;
 
-	if (!hv_vcpu)
-		return;
-
 	tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(vcpu, is_guest_mode);
+	if (!tlb_flush_fifo)
+		return;
 
 	spin_lock(&tlb_flush_fifo->write_lock);
 
@@ -1972,15 +1970,16 @@ static void hv_tlb_flush_enqueue(struct kvm_vcpu *vcpu, u64 *entries, int count,
 int kvm_hv_vcpu_flush_tlb(struct kvm_vcpu *vcpu)
 {
 	struct kvm_vcpu_hv_tlb_flush_fifo *tlb_flush_fifo;
-	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
 	u64 entries[KVM_HV_TLB_FLUSH_FIFO_SIZE];
 	int i, j, count;
 	gva_t gva;
 
-	if (!tdp_enabled || !hv_vcpu)
+	if (!tdp_enabled)
 		return -EINVAL;
 
 	tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(vcpu, is_guest_mode(vcpu));
+	if (!tlb_flush_fifo)
+		return -EINVAL;
 
 	count = kfifo_out(&tlb_flush_fifo->entries, entries, KVM_HV_TLB_FLUSH_FIFO_SIZE);
 
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 65e89ed65349..e6c74cfbb1cb 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -201,6 +201,9 @@ static inline struct kvm_vcpu_hv_tlb_flush_fifo *kvm_hv_get_tlb_flush_fifo(struc
 	int i = is_guest_mode ? HV_L2_TLB_FLUSH_FIFO :
 				HV_L1_TLB_FLUSH_FIFO;
 
+	if (!hv_vcpu)
+		return NULL;
+
 	return &hv_vcpu->tlb_flush_fifo[i];
 }
 
@@ -208,10 +211,12 @@ static inline void kvm_hv_vcpu_purge_flush_tlb(struct kvm_vcpu *vcpu)
 {
 	struct kvm_vcpu_hv_tlb_flush_fifo *tlb_flush_fifo;
 
-	if (!to_hv_vcpu(vcpu) || !kvm_check_request(KVM_REQ_HV_TLB_FLUSH, vcpu))
+	if (!kvm_check_request(KVM_REQ_HV_TLB_FLUSH, vcpu))
 		return;
 
 	tlb_flush_fifo = kvm_hv_get_tlb_flush_fifo(vcpu, is_guest_mode(vcpu));
+	if (!tlb_flush_fifo)
+		return;
 
 	kfifo_reset_out(&tlb_flush_fifo->entries);
 }
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 1/8] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 2/8] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo() Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:22   ` sashiko-bot
  2026-06-12 23:06 ` [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

When initializing a vCPU's Hyper-V object, ensure the object is fully
initialized prior to exposing it through the vCPU, and ensure accesses from
other tasks (e.g. other vCPUs) see the fully initialized object if
vcpu->arch.hyperv is non-NULL.

Lack of ordering manifests as a lockdep splat due to attempting to lock a
TLB flush FIFO before the spinlock is initialized.

  INFO: trying to register non-static key.
  The code is fine but needs lockdep annotation, or maybe
  you didn't initialize this object before use?
  turning off the locking correctness validator.
  CPU: 1 PID: 5005 Comm: syz-executor189 Not tainted 6.6.120-smp-DEV #1
  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/18/2026
  Call Trace:
   <TASK>
    [<ffffffff810dd10c>] dump_stack_lvl+0xcc/0x130 lib/dump_stack.c:106
    [<ffffffff8192bddd>] assign_lock_key+0x1fd/0x230 kernel/locking/lockdep.c:977
    [<ffffffff8191cb97>] register_lock_class+0x187/0x7a0 kernel/locking/lockdep.c:1291
    [<ffffffff8191e7a9>] __lock_acquire+0x179/0x7650 kernel/locking/lockdep.c:5016
    [<ffffffff8191e28f>] lock_acquire+0x13f/0x3d0 kernel/locking/lockdep.c:5756
    [<ffffffff8101a65b>] __raw_spin_lock include/linux/spinlock_api_smp.h:133 [inline]
    [<ffffffff8101a65b>] _raw_spin_lock+0x2b/0x40 kernel/locking/spinlock.c:154
    [<ffffffff81319d44>] spin_lock include/linux/spinlock.h:351 [inline]
    [<ffffffff81319d44>] hv_tlb_flush_enqueue+0xb4/0x270 arch/x86/kvm/hyperv.c:1946
    [<ffffffff813160c6>] kvm_hv_flush_tlb+0xa96/0x1dc0 arch/x86/kvm/hyperv.c:2145
    [<ffffffff8131438b>] kvm_hv_hypercall+0x103b/0x1fe0 arch/x86/kvm/hyperv.c:-1
    [<ffffffff8133bff3>] __vmx_handle_exit arch/x86/kvm/vmx/vmx.c:6624 [inline]
    [<ffffffff8133bff3>] vmx_handle_exit+0x12e3/0x21f0 arch/x86/kvm/vmx/vmx.c:6641
    [<ffffffff81215d11>] vcpu_enter_guest arch/x86/kvm/x86.c:11649 [inline]
    [<ffffffff81215d11>] vcpu_run+0x4d01/0x79c0 arch/x86/kvm/x86.c:11832
    [<ffffffff8120fe39>] kvm_arch_vcpu_ioctl_run+0xb49/0x1c80 arch/x86/kvm/x86.c:12179
    [<ffffffff8119cd60>] kvm_vcpu_ioctl+0xc80/0xff0 virt/kvm/kvm_main.c:6029
    [<ffffffff8226fefd>] vfs_ioctl fs/ioctl.c:52 [inline]
    [<ffffffff8226fefd>] __do_sys_ioctl fs/ioctl.c:872 [inline]
    [<ffffffff8226fefd>] __se_sys_ioctl+0xfd/0x170 fs/ioctl.c:858
    [<ffffffff85ac97d9>] do_syscall_x64 arch/x86/entry/common.c:52 [inline]
    [<ffffffff85ac97d9>] do_syscall_64+0x69/0xb0 arch/x86/entry/common.c:93
   [<ffffffff85c000d0>] entry_SYSCALL_64_after_hwframe+0x68/0xd2
   </TASK>

Use the "safe" variant in all paths that are known to access the Hyper-V
object, as detected by an upcoming lockdep assertion.

Fixes: 0823570f0198 ("KVM: x86: hyper-v: Introduce TLB flush fifo")
Fixes: fc08b628d7c9 ("KVM: x86: hyper-v: Allocate Hyper-V context lazily")
Reported-by: syzbot+5b32c49cd8f005e65654@syzkaller.appspotmail.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 23 ++++++++++++++++++-----
 arch/x86/kvm/hyperv.h | 16 ++++++++++++++--
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index ecd344b91739..107eb7df20f1 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -206,13 +206,19 @@ static struct kvm_vcpu *get_vcpu_by_vpidx(struct kvm *kvm, u32 vpidx)
 
 static struct kvm_vcpu_hv_synic *synic_get(struct kvm *kvm, u32 vpidx)
 {
-	struct kvm_vcpu *vcpu;
 	struct kvm_vcpu_hv_synic *synic;
+	struct kvm_vcpu_hv *hv_vcpu;
+	struct kvm_vcpu *vcpu;
 
 	vcpu = get_vcpu_by_vpidx(kvm, vpidx);
-	if (!vcpu || !to_hv_vcpu(vcpu))
+	if (!vcpu)
 		return NULL;
-	synic = to_hv_synic(vcpu);
+
+	hv_vcpu = to_hv_vcpu_safe(vcpu);
+	if (!hv_vcpu)
+		return NULL;
+
+	synic = &hv_vcpu->synic;
 	return (synic->active) ? synic : NULL;
 }
 
@@ -972,7 +978,6 @@ int kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
 	if (!hv_vcpu)
 		return -ENOMEM;
 
-	vcpu->arch.hyperv = hv_vcpu;
 	hv_vcpu->vcpu = vcpu;
 
 	synic_init(&hv_vcpu->synic);
@@ -988,6 +993,14 @@ int kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
 		spin_lock_init(&hv_vcpu->tlb_flush_fifo[i].write_lock);
 	}
 
+	/*
+	 * Ensure the structure is fully initialized before it's visible to
+	 * other tasks, as much of the state can be legally accessed without
+	 * holding vcpu->mutex.
+	 *
+	 * Pairs with the smp_load_acquire() in to_hv_vcpu_safe().
+	 */
+	smp_store_release(&vcpu->arch.hyperv, hv_vcpu);
 	return 0;
 }
 
@@ -2166,7 +2179,7 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 		bitmap_zero(vcpu_mask, KVM_MAX_VCPUS);
 
 		kvm_for_each_vcpu(i, v, kvm) {
-			hv_v = to_hv_vcpu(v);
+			hv_v = to_hv_vcpu_safe(v);
 
 			/*
 			 * The following check races with nested vCPUs entering/exiting
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index e6c74cfbb1cb..821b586ed746 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -61,6 +61,18 @@ static inline struct kvm_hv *to_kvm_hv(struct kvm *kvm)
 	return &kvm->arch.hyperv;
 }
 
+static inline struct kvm_vcpu_hv *to_hv_vcpu_safe(struct kvm_vcpu *vcpu)
+{
+	/*
+	 * Ensure the HyperV structure is fully initialized when accessing it
+	 * without holding vcpu->mutex (or some other guarantee that KVM can't
+	 * concurrently instantiate the structure).
+	 *
+	 * Pairs with the smp_store_release() in kvm_hv_vcpu_init().
+	 */
+	return smp_load_acquire(&vcpu->arch.hyperv);
+}
+
 static inline struct kvm_vcpu_hv *to_hv_vcpu(struct kvm_vcpu *vcpu)
 {
 	return vcpu->arch.hyperv;
@@ -87,7 +99,7 @@ static inline struct kvm_hv_syndbg *to_hv_syndbg(struct kvm_vcpu *vcpu)
 
 static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu)
 {
-	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
+	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu_safe(vcpu);
 
 	return hv_vcpu ? hv_vcpu->vp_index : vcpu->vcpu_idx;
 }
@@ -197,7 +209,7 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 static inline struct kvm_vcpu_hv_tlb_flush_fifo *kvm_hv_get_tlb_flush_fifo(struct kvm_vcpu *vcpu,
 									   bool is_guest_mode)
 {
-	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
+	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu_safe(vcpu);
 	int i = is_guest_mode ? HV_L2_TLB_FLUSH_FIFO :
 				HV_L1_TLB_FLUSH_FIFO;
 
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
                   ` (2 preceding siblings ...)
  2026-06-12 23:06 ` [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:30   ` sashiko-bot
  2026-06-12 23:06 ` [PATCH v2 5/8] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper Sean Christopherson
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

Invalidate a vCPU's index immediately after allocating storage for the vCPU
so that KVM doesn't incorrectly treat a vCPU that is the process of being
created as being vCPU0.  This will also allow detecting that a vCPU is in
the process of being created and thus otherwise unreachable, which is
useful for avoiding false positives in lockdep assertions on vcpu->mutex.

Note, kvm_wait_for_vcpu_online() naturally does the right thing thanks to
vcpu->vcpu_idx and kvm->online_vcpus being signed values.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 virt/kvm/kvm_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e44c20c04961..b3d2a678210c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4188,6 +4188,8 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
 		goto vcpu_decrement;
 	}
 
+	vcpu->vcpu_idx = -1;
+
 	BUILD_BUG_ON(sizeof(struct kvm_run) > PAGE_SIZE);
 	page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
 	if (!page) {
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/8] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
                   ` (3 preceding siblings ...)
  2026-06-12 23:06 ` [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 6/8] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

Extract nVMX's lockdep assertion that a vCPU is locked or otherwise
unreachable into a common helper, as KVM x86 is about to gain another user,
but there is nothing x86-specific about the logic, i.e. the assertion may
be useful for other architectures.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.h | 6 ++----
 include/linux/kvm_host.h  | 6 ++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index 6d6cd5904ddf..c6de848bd9ce 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -57,16 +57,14 @@ bool nested_vmx_check_io_bitmaps(struct kvm_vcpu *vcpu, unsigned int port,
 
 static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
 {
-	lockdep_assert_once(lockdep_is_held(&vcpu->mutex) ||
-			    !refcount_read(&vcpu->kvm->users_count));
+	kvm_lockdep_assert_vcpu_is_locked_or_unreachable(vcpu);
 
 	return to_vmx(vcpu)->nested.cached_vmcs12;
 }
 
 static inline struct vmcs12 *get_shadow_vmcs12(struct kvm_vcpu *vcpu)
 {
-	lockdep_assert_once(lockdep_is_held(&vcpu->mutex) ||
-			    !refcount_read(&vcpu->kvm->users_count));
+	kvm_lockdep_assert_vcpu_is_locked_or_unreachable(vcpu);
 
 	return to_vmx(vcpu)->nested.cached_shadow_vmcs12;
 }
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 27498e990dff..82696b27b145 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -989,6 +989,12 @@ static inline struct kvm_io_bus *kvm_get_bus(struct kvm *kvm, enum kvm_bus idx)
 					 lockdep_is_held(&kvm->slots_lock));
 }
 
+static inline void kvm_lockdep_assert_vcpu_is_locked_or_unreachable(struct kvm_vcpu *vcpu)
+{
+	lockdep_assert_once(lockdep_is_held(&vcpu->mutex) ||
+			    !refcount_read(&vcpu->kvm->users_count));
+}
+
 static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
 {
 	int num_vcpus = atomic_read(&kvm->online_vcpus);
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/8] KVM: x86: Treat a vCPU as unreachable if its index is invalid
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
                   ` (4 preceding siblings ...)
  2026-06-12 23:06 ` [PATCH v2 5/8] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 7/8] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 8/8] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses Sean Christopherson
  7 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

In the "vCPU locked or unreachable" lockdep assertion, treat a vCPU as
unreachable if its index is invalid, i.e. if the vCPU is in the process of
being created.  Until the vCPU is inserted into the array of vCPUs, the
only way to get at the vCPU is via kvm_vm_ioctl_create_vcpu().  Note, the
actual index is set _before_ adding the vCPU to the array, i.e. there's no
risk of a false negative on the lockdep assertion.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 include/linux/kvm_host.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 82696b27b145..017070b81108 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -992,6 +992,7 @@ static inline struct kvm_io_bus *kvm_get_bus(struct kvm *kvm, enum kvm_bus idx)
 static inline void kvm_lockdep_assert_vcpu_is_locked_or_unreachable(struct kvm_vcpu *vcpu)
 {
 	lockdep_assert_once(lockdep_is_held(&vcpu->mutex) ||
+			    vcpu->vcpu_idx < 0 ||
 			    !refcount_read(&vcpu->kvm->users_count));
 }
 
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 7/8] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
                   ` (5 preceding siblings ...)
  2026-06-12 23:06 ` [PATCH v2 6/8] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  2026-06-12 23:06 ` [PATCH v2 8/8] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses Sean Christopherson
  7 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

Assert that either vcpu->mutex is held or the VM is otherwise unreachable
when using the normal vCPU => HyperV accessor to help detect improper
cross-task usage of the HyperV structure.  When accessing the structure
without holding the vCPU's mutex, e.g. to send interrupts or to queue TLB
flushes, KVM needs to use the more paranoid to_hv_vcpu_safe() to guarantee
that it can't see a half-baked structure.

To avoid false positives, open code accesses to vcpu->arch.hyperv in the
Synthetic Timer callbacks (can be reached if and only if HyperV state is
fully initialized).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 6 ++----
 arch/x86/kvm/hyperv.h | 2 ++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 107eb7df20f1..7efe2907148f 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -599,8 +599,7 @@ static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
 {
 	struct kvm_vcpu *vcpu = hv_stimer_to_vcpu(stimer);
 
-	set_bit(stimer->index,
-		to_hv_vcpu(vcpu)->stimer_pending_bitmap);
+	set_bit(stimer->index, vcpu->arch.hyperv->stimer_pending_bitmap);
 	kvm_make_request(KVM_REQ_HV_STIMER, vcpu);
 	if (vcpu_kick)
 		kvm_vcpu_kick(vcpu);
@@ -614,8 +613,7 @@ static void stimer_cleanup(struct kvm_vcpu_hv_stimer *stimer)
 				    stimer->index);
 
 	hrtimer_cancel(&stimer->timer);
-	clear_bit(stimer->index,
-		  to_hv_vcpu(vcpu)->stimer_pending_bitmap);
+	clear_bit(stimer->index, vcpu->arch.hyperv->stimer_pending_bitmap);
 	stimer->msg_pending = false;
 	stimer->exp_time = 0;
 }
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 821b586ed746..f78ab3c8d11a 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -75,6 +75,8 @@ static inline struct kvm_vcpu_hv *to_hv_vcpu_safe(struct kvm_vcpu *vcpu)
 
 static inline struct kvm_vcpu_hv *to_hv_vcpu(struct kvm_vcpu *vcpu)
 {
+	kvm_lockdep_assert_vcpu_is_locked_or_unreachable(vcpu);
+
 	return vcpu->arch.hyperv;
 }
 
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 8/8] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses
  2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
                   ` (6 preceding siblings ...)
  2026-06-12 23:06 ` [PATCH v2 7/8] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
@ 2026-06-12 23:06 ` Sean Christopherson
  7 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654

When activating Hyper-V's Synthetic Interrupt Controller (SynIC), mark it
active with WRITE_ONCE() and query it using READ_ONCE() in synic_get(),
the only known cross-task reader, to document that the flag is accessed
without holding the vCPU's mutex.

Note, there are no data dependencies on the SynIC being marked active,
e.g. the vector read by synic_set_irq() is set (usually in response to
guest activity) long after the SynIC is initially activated, and a false
negative on the SynIC being active would be benign (ignoring that such a
race is likely to be problematic for the guest irrespective of what KVM
does).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 7efe2907148f..63754a62dc87 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -219,7 +219,7 @@ static struct kvm_vcpu_hv_synic *synic_get(struct kvm *kvm, u32 vpidx)
 		return NULL;
 
 	synic = &hv_vcpu->synic;
-	return (synic->active) ? synic : NULL;
+	return READ_ONCE(synic->active) ? synic : NULL;
 }
 
 static void kvm_hv_notify_acked_sint(struct kvm_vcpu *vcpu, u32 sint)
@@ -1013,7 +1013,7 @@ int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages)
 
 	synic = to_hv_synic(vcpu);
 
-	synic->active = true;
+	WRITE_ONCE(synic->active, true);
 	synic->dont_zero_synic_pages = dont_zero_synic_pages;
 	synic->control = HV_SYNIC_CONTROL_ENABLE;
 	return 0;
-- 
2.54.0.1136.gdb2ca164c4-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses
  2026-06-12 23:06 ` [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
@ 2026-06-12 23:22   ` sashiko-bot
  2026-06-13  0:20     ` Sean Christopherson
  0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-06-12 23:22 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm

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

Pre-existing issues:
- [High] Incomplete conversion to safe Hyper-V object accessor causes a false positive lockdep splat and unsafe cross-vCPU memory reads.
--

commit c312f67e4d3ecc2c19765c4ac40bd23b51d526ce
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses

This commit ensures the vCPU's Hyper-V object is fully initialized before
exposing it to other tasks by using smp_store_release(). It also introduces
and applies a safe to_hv_vcpu_safe() accessor using smp_load_acquire() to
safely retrieve the object in cross-vCPU contexts.

> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index e6c74cfbb1cb6..821b586ed7464 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -61,6 +61,18 @@ static inline struct kvm_hv *to_kvm_hv(struct kvm *kvm)
>  	return &kvm->arch.hyperv;
>  }
>  
> +static inline struct kvm_vcpu_hv *to_hv_vcpu_safe(struct kvm_vcpu *vcpu)
> +{
> +	/*
> +	 * Ensure the HyperV structure is fully initialized when accessing it
> +	 * without holding vcpu->mutex (or some other guarantee that KVM can't
> +	 * concurrently instantiate the structure).
> +	 *
> +	 * Pairs with the smp_store_release() in kvm_hv_vcpu_init().
> +	 */
> +	return smp_load_acquire(&vcpu->arch.hyperv);
> +}
> +

[Severity: High]
This isn't a bug introduced by this patch, but should 
kvm_hv_has_stimer_pending() also be updated to use to_hv_vcpu_safe()?

It currently still uses the unsafe to_hv_vcpu() accessor:

static inline bool kvm_hv_has_stimer_pending(struct kvm_vcpu *vcpu)
{
	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
	...

Since kvm_hv_has_stimer_pending() can be reached cross-vCPU without holding
the target vcpu->mutex via the spin-loop yield path:

kvm_vcpu_on_spin() -> vcpu_dy_runnable() -> kvm_arch_dy_runnable() ->
kvm_arch_vcpu_runnable() -> kvm_vcpu_has_events() ->
kvm_hv_has_stimer_pending()

Could this bypass the necessary memory barriers and result in reading an
uninitialized stimer_pending_bitmap?

Also, won't using the unsafe accessor here unconditionally trigger the lockdep
assertion introduced later in the series when accessed cross-vCPU?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612230622.687665-1-seanjc@google.com?part=3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created
  2026-06-12 23:06 ` [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
@ 2026-06-12 23:30   ` sashiko-bot
  2026-06-12 23:40     ` Sean Christopherson
  0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-06-12 23:30 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Setting `vcpu_idx` to `-1` changes the default initialization of Xen `vcpu_id` from `0` to `-1`, potentially breaking VMMs that rely on the implicit default for vCPU 0.
--

commit bcf5ca6a21c901bf1c4eeb8dcb3b2d169932ed92
Author: Sean Christopherson <seanjc@google.com>

KVM: Initialize a vCPU's index to '-1' while it's being created

This commit invalidates a vCPU's index immediately after allocating storage
for the vCPU. This prevents KVM from incorrectly treating an incomplete vCPU
as vCPU0 and allows detecting unreachable vCPUs.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e44c20c049610..b3d2a678210c3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4188,6 +4188,8 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>  		goto vcpu_decrement;
>  	}
>  
> +	vcpu->vcpu_idx = -1;
> +

[Severity: High]
Does setting vcpu->vcpu_idx to -1 here inadvertently change the default
initialization of Xen's vcpu_id?

Because this is set before kvm_arch_vcpu_create(), the architecture-specific
creation code will see the -1 value. On x86, kvm_arch_vcpu_create()
eventually calls kvm_xen_init_vcpu():

arch/x86/kvm/xen.c:kvm_xen_init_vcpu() {
    ...
    vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
    ...
}

Since the actual vcpu_idx isn't assigned until later in
kvm_vm_ioctl_create_vcpu(), this leaves vcpu->arch.xen.vcpu_id as -1 instead
of its previous default of 0.

Could this cause guest hypercalls, such as VCPUOP_set_singleshot_timer,
to fail with -EINVAL if a VMM relies on the previous default of 0 for vCPU 0
and doesn't explicitly invoke KVM_XEN_VCPU_SET_ATTR?

>  	BUILD_BUG_ON(sizeof(struct kvm_run) > PAGE_SIZE);
>  	page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
>  	if (!page) {

[ ... ]

>  	vcpu->run = page_address(page);
>  
>  	kvm_vcpu_init(vcpu, kvm, id);
>  
>  	r = kvm_arch_vcpu_create(vcpu);
>  	if (r)
>  		goto vcpu_free_run_page;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612230622.687665-1-seanjc@google.com?part=4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created
  2026-06-12 23:30   ` sashiko-bot
@ 2026-06-12 23:40     ` Sean Christopherson
  2026-06-13  0:49       ` David Woodhouse
  0 siblings, 1 reply; 14+ messages in thread
From: Sean Christopherson @ 2026-06-12 23:40 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm, Paul Durrant, David Woodhouse

+David and Paul

On Fri, Jun 12, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index e44c20c049610..b3d2a678210c3 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -4188,6 +4188,8 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
> >  		goto vcpu_decrement;
> >  	}
> >  
> > +	vcpu->vcpu_idx = -1;
> > +
> 
> [Severity: High]
> Does setting vcpu->vcpu_idx to -1 here inadvertently change the default
> initialization of Xen's vcpu_id?

Gah, I spent the better part of an hour going through various architecture code
to make sure nothing consumed vcpu_idx in the vCPU creation paths, and still
missed this.

> Because this is set before kvm_arch_vcpu_create(), the architecture-specific
> creation code will see the -1 value. On x86, kvm_arch_vcpu_create()
> eventually calls kvm_xen_init_vcpu():
> 
> arch/x86/kvm/xen.c:kvm_xen_init_vcpu() {
>     ...
>     vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;

I don't see how the existing code can be correct.  David/Paul, is this supposed
to be vcpu->vcpu_id?

>     ...
> }
> 
> Since the actual vcpu_idx isn't assigned until later in
> kvm_vm_ioctl_create_vcpu(), this leaves vcpu->arch.xen.vcpu_id as -1 instead
> of its previous default of 0.
> 
> Could this cause guest hypercalls, such as VCPUOP_set_singleshot_timer,
> to fail with -EINVAL if a VMM relies on the previous default of 0 for vCPU 0
> and doesn't explicitly invoke KVM_XEN_VCPU_SET_ATTR?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses
  2026-06-12 23:22   ` sashiko-bot
@ 2026-06-13  0:20     ` Sean Christopherson
  0 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2026-06-13  0:20 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm

On Fri, Jun 12, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> > index e6c74cfbb1cb6..821b586ed7464 100644
> > --- a/arch/x86/kvm/hyperv.h
> > +++ b/arch/x86/kvm/hyperv.h
> > @@ -61,6 +61,18 @@ static inline struct kvm_hv *to_kvm_hv(struct kvm *kvm)
> >  	return &kvm->arch.hyperv;
> >  }
> >  
> > +static inline struct kvm_vcpu_hv *to_hv_vcpu_safe(struct kvm_vcpu *vcpu)
> > +{
> > +	/*
> > +	 * Ensure the HyperV structure is fully initialized when accessing it
> > +	 * without holding vcpu->mutex (or some other guarantee that KVM can't
> > +	 * concurrently instantiate the structure).
> > +	 *
> > +	 * Pairs with the smp_store_release() in kvm_hv_vcpu_init().
> > +	 */
> > +	return smp_load_acquire(&vcpu->arch.hyperv);
> > +}
> > +
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but should 
> kvm_hv_has_stimer_pending() also be updated to use to_hv_vcpu_safe()?
> 
> It currently still uses the unsafe to_hv_vcpu() accessor:
> 
> static inline bool kvm_hv_has_stimer_pending(struct kvm_vcpu *vcpu)
> {
> 	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
> 	...
> 
> Since kvm_hv_has_stimer_pending() can be reached cross-vCPU without holding
> the target vcpu->mutex via the spin-loop yield path:
> 
> kvm_vcpu_on_spin() -> vcpu_dy_runnable() -> kvm_arch_dy_runnable() ->
> kvm_arch_vcpu_runnable() -> kvm_vcpu_has_events() ->
> kvm_hv_has_stimer_pending()
> 
> Could this bypass the necessary memory barriers and result in reading an
> uninitialized stimer_pending_bitmap?
> 
> Also, won't using the unsafe accessor here unconditionally trigger the lockdep
> assertion introduced later in the series when accessed cross-vCPU?

*sigh*

Yes and yes.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created
  2026-06-12 23:40     ` Sean Christopherson
@ 2026-06-13  0:49       ` David Woodhouse
  0 siblings, 0 replies; 14+ messages in thread
From: David Woodhouse @ 2026-06-13  0:49 UTC (permalink / raw)
  To: Sean Christopherson, sashiko-reviews; +Cc: kvm, Paul Durrant

[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]

On Fri, 2026-06-12 at 16:40 -0700, Sean Christopherson wrote:
> > 
> > arch/x86/kvm/xen.c:kvm_xen_init_vcpu() {
> >     ...
> >     vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
> 
> I don't see how the existing code can be correct.  David/Paul, is this supposed
> to be vcpu->vcpu_id?

No, vcpu->vcpu_id is the APIC ID. 

But xen.vcpu_id is the Xen/ACPI ID. The kernel needs to know it in
order to accelerate timer hypercalls. The vcpu->vcpu_idx is a
reasonable default for that.

(As if this wasn't confusing enough, note ACPI != APIC and neither of
those are typos)

> >     ...
> > }
> > 
> > Since the actual vcpu_idx isn't assigned until later in
> > kvm_vm_ioctl_create_vcpu(), this leaves vcpu->arch.xen.vcpu_id as -1 instead
> > of its previous default of 0.
> > 
> > Could this cause guest hypercalls, such as VCPUOP_set_singleshot_timer,
> > to fail with -EINVAL if a VMM relies on the previous default of 0 for vCPU 0
> > and doesn't explicitly invoke KVM_XEN_VCPU_SET_ATTR?

Yeah, but it would be tolerable to just return !handled for the -1 case
instead of "I handled that and it got -EINVAL". Then the hypercall gets
punted to userspace.

I'd slightly prefer a deferred initialization, but I believe all known
userspace explicitly *sets* the ID via KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID
so I think letting it default to -1 would be tolerable as long as we
fix the timer hypercall.

(Huh... VCPUOP_set_singleshot_timer takes a vCPU arg but then just
returns -EINVAL if it isn't yourself? That's... special. But if that's
how I did it then I'm fairly sure it's a faithful representation of
what Xen does. Might look that up in the morning, but then again we've
launched over 5 billion guests with it like that and nobody's whined at
me yet...)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-06-13  0:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 23:06 [PATCH v2 0/8] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 1/8] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 2/8] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo() Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 3/8] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
2026-06-12 23:22   ` sashiko-bot
2026-06-13  0:20     ` Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 4/8] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
2026-06-12 23:30   ` sashiko-bot
2026-06-12 23:40     ` Sean Christopherson
2026-06-13  0:49       ` David Woodhouse
2026-06-12 23:06 ` [PATCH v2 5/8] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 6/8] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 7/8] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
2026-06-12 23:06 ` [PATCH v2 8/8] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox