[PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv

[PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv

[Sean Christopherson]

Fix a bug found by syzkaller (originally on a Google-internal kernel, but now
on upstream as well) 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.

v3:
 - Reset vcpu_idx back to -1 if adding the vCPU to the xarray fails. [syzbot]
 - Use the safe accessor in kvm_hv_has_stimer_pending(). [sashiko]
 - Explicitly initialize vcpu->arch.xen.vcpu_id to XEN_VCPU_ID_INVALID, and
   punt singleshot timer hypercalls to userspace if the vCPU ID hasn't been
   set. [sashiko, David]

v2:
 - https://lore.kernel.org/all/20260612230622.687665-1-seanjc@google.com
 - 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 (10):
  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: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID
    isn't set
  KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer
    hypercalls
  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     | 27 ++++++++++++++---
 arch/x86/kvm/vmx/nested.h |  6 ++--
 arch/x86/kvm/xen.c        | 20 ++++++------
 include/linux/kvm_host.h  |  7 +++++
 virt/kvm/kvm_main.c       | 11 ++++++-
 6 files changed, 86 insertions(+), 49 deletions(-)


base-commit: a204badd8432f93b7e862e7dac6db0fe3d65f370
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 01/10] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller

[Sean Christopherson]

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 1ee0d23f8949..2dc3e64b3f2f 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
@@ -2144,11 +2145,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)) {
@@ -2158,9 +2156,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);
@@ -2191,9 +2187,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.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 02/10] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo()

[Sean Christopherson]

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 2dc3e64b3f2f..49b1154366ce 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 1c8f7aaab063..2da11b967c41 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -202,6 +202,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];
 }
 
@@ -209,10 +212,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.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 03/10] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses

[Sean Christopherson]

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, with an assist or two
from Sashiko.

Link: https://lore.kernel.org/all/20260612232258.0D9131F000E9@smtp.kernel.org
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
Reported-by: syzbot+5d2b94b77112148d1744@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a396a66.52ae72c2.136ac7.0002.GAE@google.com
Tested-by: syzbot+5d2b94b77112148d1744@syzkaller.appspotmail.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 23 ++++++++++++++++++-----
 arch/x86/kvm/hyperv.h | 18 +++++++++++++++---
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 49b1154366ce..888526ce4dab 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;
 }
 
@@ -2165,7 +2178,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 2da11b967c41..ea9c81d76dd3 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -62,6 +62,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;
@@ -88,7 +100,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;
 }
@@ -142,7 +154,7 @@ static inline struct kvm_vcpu *hv_stimer_to_vcpu(struct kvm_vcpu_hv_stimer *stim
 
 static inline bool kvm_hv_has_stimer_pending(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);
 
 	if (!hv_vcpu)
 		return false;
@@ -198,7 +210,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.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set

[Sean Christopherson]

Explicitly invalidate KVM's internal Xen vCPU ID during vCPU creation
instead of *trying* to set the Xen ID to the vCPU index by default, and
forward singleshot timer hypercalls to userspace if the VMM hasn't set the
Xen ID via KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.  Using the vCPU's index as its
default Xen ID is reasonable in concept, but in practice is horribly flawed
as the index is left as '0' until after vCPU initialization completes, i.e.
every vCPU gets a Xen ID of '0' by default.

Forward hypercalls to userspace instead of trying to salvage any kind of
default behavior, as all userspace implementations that support multiple
vCPUs either don't enable the timer, are guaranteed to set Xen ID, or work
only because *all* guests also screw up the singleshot timer hypercalls.
The last scenarios is extremely unlikely given that Linux-as-a-guest uses
the actual Xen vCPU ID when making timer hypercalls.  In other words, for
all intents and purposes, KVM's ABI is already that userspace must set the
Xen vCPU ID, so just commit to that ABI.

Note, KVM's handling of KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID restricts the ID to
KVM_MAX_VCPUS, so there's no chance of a valid ID colliding with -1u.

Link: https://lore.kernel.org/all/20260612233017.1F9771F000E9@smtp.kernel.org
Suggested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/xen.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 694b31c1fcc9..27c1aeeab8af 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -22,6 +22,7 @@
 #include <xen/interface/version.h>
 #include <xen/interface/event_channel.h>
 #include <xen/interface/sched.h>
+#include <xen/xen-ops.h>
 
 #include <asm/xen/cpuid.h>
 #include <asm/pvclock.h>
@@ -1610,6 +1611,9 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
 	if (!kvm_xen_timer_enabled(vcpu))
 		return false;
 
+	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
+		return false;
+
 	switch (cmd) {
 	case VCPUOP_set_singleshot_timer:
 		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
@@ -2299,7 +2303,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
 
 void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
 {
-	vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
+	vcpu->arch.xen.vcpu_id = XEN_VCPU_ID_INVALID;
 	vcpu->arch.xen.poll_evtchn = 0;
 
 	timer_setup(&vcpu->arch.xen.poll_timer, cancel_evtchn_poll, 0);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

[Sean Christopherson]

Hoist the checks on the Xen vCPU ID when handling set_singleshot_timer and
stop_singleshot_timer hypercalls out of their individual case-statements,
so that both checks on the ID are in common code.  kvm_xen_hcall_vcpu_op()
is already doubly committed to handling only singleshot timer hypercalls,
and even if that were to change in the future, the function could simply
be renamed and turned into a helper specifically for timer hypercalls.

No functional change intended.

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

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 27c1aeeab8af..db10f12d10cf 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1614,13 +1614,13 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
 	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
 		return false;
 
+	if (vcpu->arch.xen.vcpu_id != vcpu_id) {
+		*r = -EINVAL;
+		return true;
+	}
+
 	switch (cmd) {
 	case VCPUOP_set_singleshot_timer:
-		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
-			*r = -EINVAL;
-			return true;
-		}
-
 		/*
 		 * The only difference for 32-bit compat is the 4 bytes of
 		 * padding after the interesting part of the structure. So
@@ -1648,10 +1648,6 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
 		return true;
 
 	case VCPUOP_stop_singleshot_timer:
-		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
-			*r = -EINVAL;
-			return true;
-		}
 		kvm_xen_stop_timer(vcpu);
 		*r = 0;
 		return true;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created

[Sean Christopherson]

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.

Unwind the index back to -1 if insert the vCPU into the array fails so that
kvm_arch_vcpu_destroy() sees the vCPU as unreachable, i.e. so that teardown
logic doesn't hit false positive lockdep assertions.  Opportunistically add
a comment to call out that the "real" index needs to be set before making
the vCPU visible to other tasks.

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 | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e44c20c04961..98da4c889ffc 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) {
@@ -4216,11 +4218,18 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
 		goto unlock_vcpu_destroy;
 	}
 
+	/*
+	 * Set the vCPU's index *before* the vCPU is reachable by other tasks.
+	 * Unwind the index back to -1 on failure so that KVM can use the index
+	 * to detect that the vCPU is unreachable, e.g. for lockdep asserts.
+	 */
 	vcpu->vcpu_idx = atomic_read(&kvm->online_vcpus);
 	r = xa_insert(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, GFP_KERNEL_ACCOUNT);
 	WARN_ON_ONCE(r == -EBUSY);
-	if (r)
+	if (r) {
+		vcpu->vcpu_idx = -1;
 		goto unlock_vcpu_destroy;
+	}
 
 	/*
 	 * Now it's all set up, let userspace reach it.  Grab the vCPU's mutex
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 07/10] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper

[Sean Christopherson]

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 ab8cfaec82d3..b10814f99a50 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.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid

[Sean Christopherson]

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 b10814f99a50..0bdfa3699352 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.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()

[Sean Christopherson]

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 888526ce4dab..f765c3bb9b1f 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 ea9c81d76dd3..37a0bcf03e28 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -76,6 +76,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.55.0.rc0.799.gd6f94ed593-goog

[PATCH v3 10/10] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses

[Sean Christopherson]

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 f765c3bb9b1f..9d38cb644668 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.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hoisting the vCPU ID check in `kvm_xen_hcall_vcpu_op()` incorrectly rejects all cross-vCPU `__HYPERVISOR_vcpu_op` hypercalls, breaking SMP initialization for Xen guests.
--

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

KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

This commit hoists the Xen vCPU ID checks for set_singleshot_timer and
stop_singleshot_timer out of their switch cases in kvm_xen_hcall_vcpu_op()
into the common path.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1614,13 +1614,13 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
>  		return false;
>  
> +	if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> +		*r = -EINVAL;
> +		return true;
> +	}
> +

[Severity: High]
Does hoisting this check unintentionally reject cross-vCPU commands that should
be delegated to userspace?

If a guest issues a cross-vCPU hypercall like VCPUOP_up or VCPUOP_initialise,
this check will now evaluate to true and return -EINVAL directly from the
kernel.

Previously, these non-timer hypercalls would bypass the switch statement and
fall through to return false, properly delegating the operation to userspace.

Could this cause regressions in SMP initialization for Xen guests by
intercepting and rejecting commands intended for secondary vCPUs?

>  	switch (cmd) {
>  	case VCPUOP_set_singleshot_timer:
> -		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> -			*r = -EINVAL;
> -			return true;
> -		}
> -

[ ... ]

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

Re: [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] False-positive lockdep assertion failure during vCPU creation error path.
--

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

KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()

This commit adds an assertion to verify that either the vCPU's mutex is held
or the VM is unreachable when using to_hv_vcpu(). It also open-codes accesses
in the Synthetic Timer callbacks to avoid false positives.

[ ... ]

> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index ea9c81d76dd38..37a0bcf03e281 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -76,6 +76,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);

[Severity: High]
Can this assertion trigger a false positive on the vCPU creation error path?

If create_vcpu_fd() fails in kvm_vm_ioctl_create_vcpu() (for example, if an
unprivileged user space process exhausts its file descriptors), the kernel
takes the error path:

virt/kvm/kvm_main.c:kvm_vm_ioctl_create_vcpu() {
    ...
kvm_put_xa_erase:
    mutex_unlock(&vcpu->mutex);
    kvm_put_kvm_no_destroy(kvm);
    xa_erase(&kvm->vcpu_array, vcpu->vcpu_idx);
unlock_vcpu_destroy:
    ...
    kvm_arch_vcpu_destroy(vcpu);
}

Since vcpu->vcpu_idx is not reset to -1 before calling kvm_arch_vcpu_destroy(),
the subsequent call chain of kvm_arch_vcpu_destroy() -> kvm_hv_vcpu_uninit() ->
to_hv_vcpu() will evaluate vcpu->vcpu_idx >= 0.

With the mutex unlocked and users_count > 0, wouldn't this cause the lockdep
assertion to fail, leading to a kernel warning or panic if panic_on_warn is
enabled?

> +
>  	return vcpu->arch.hyperv;
>  }

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

Re: [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete Error Handling / False Positive Lockdep splat in `kvm_vm_ioctl_create_vcpu` error path.
--

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

KVM: x86: Treat a vCPU as unreachable if its index is invalid

This commit modifies the vCPU lockdep assertion to treat vCPUs with a
negative index as unreachable, aiming to avoid false positives while the
vCPU is in the process of being created.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index b10814f99a50b..0bdfa36993527 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 ||

[Severity: High]
Does this check fully cover the failure path in kvm_vm_ioctl_create_vcpu()
when create_vcpu_fd() fails?

If an unprivileged user exhausts their file descriptor limit,
create_vcpu_fd() can fail after the vCPU is successfully inserted into the
xarray (setting vcpu->vcpu_idx >= 0). The error handler then does this:

virt/kvm/kvm_main.c:kvm_vm_ioctl_create_vcpu() {
    ...
kvm_put_xa_erase:
        mutex_unlock(&vcpu->mutex);
        kvm_put_kvm_no_destroy(kvm);
        xa_erase(&kvm->vcpu_array, vcpu->vcpu_idx);
unlock_vcpu_destroy:
        kvm_arch_vcpu_destroy(vcpu);
    ...
}

Since xa_erase() is called but vcpu->vcpu_idx is not reset to -1, the index
remains >= 0. When cleanup proceeds to kvm_arch_vcpu_destroy() and
eventually evaluates this lockdep assertion, the mutex is unlocked and
vcpu_idx >= 0.

Could this result in a false positive lockdep splat that panics the host if
panic_on_warn is enabled?

>  			    !refcount_read(&vcpu->kvm->users_count));
>  }

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

Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM returns an un-restorable value for KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID when the Xen vCPU ID is not set, breaking live migration.
- [Medium] kvm_xen_hcall_set_timer_op() handles single-shot timer hypercalls in the kernel even when the Xen vCPU ID is invalid, contradicting the commit's intent.
--

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

KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set

Explicitly invalidate KVM's internal Xen vCPU ID during vCPU creation
instead of *trying* to set the Xen ID to the vCPU index by default, and
forward singleshot timer hypercalls to userspace if the VMM hasn't set the
Xen ID via KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.  Using the vCPU's index as its
default Xen ID is reasonable in concept, but in practice is horribly flawed
as the index is left as '0' until after vCPU initialization completes, i.e.
every vCPU gets a Xen ID of '0' by default.

Link: https://lore.kernel.org/all/20260612233017.1F9771F000E9@smtp.kernel.org

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 694b31c1fcc9a..27c1aeeab8afb 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
[ ... ]
> @@ -1610,6 +1611,9 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  	if (!kvm_xen_timer_enabled(vcpu))
>  		return false;
>  
> +	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> +		return false;
> +

[Severity: Medium]
Does this bypass the userspace punting rule for the __HYPERVISOR_set_timer_op
hypercall?

While this correctly punts for kvm_xen_hcall_vcpu_op(), the older hypercall is
still handled natively without the check:

arch/x86/kvm/xen.c:kvm_xen_hcall_set_timer_op() {
    if (!kvm_xen_timer_enabled(vcpu))
        return false;

    if (timeout)
        kvm_xen_start_timer(vcpu, timeout, true);
    else
        kvm_xen_stop_timer(vcpu);

    *r = 0;
    return true;
}

If a Xen guest issues __HYPERVISOR_set_timer_op on a vCPU where the VMM hasn't
configured the ID, it is still handled in the kernel.

Will this create an inconsistent ABI where one single-shot timer hypercall
variant is punted, while the other is handled natively?

>  	switch (cmd) {
>  	case VCPUOP_set_singleshot_timer:
>  		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
[ ... ]
> @@ -2299,7 +2303,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
>  
>  void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
>  {
> -	vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
> +	vcpu->arch.xen.vcpu_id = XEN_VCPU_ID_INVALID;

[Severity: High]
Does this initialization break live migration when the ID isn't configured?

When userspace calls KVM_GET_XEN_VCPU_ATTR, it retrieves the uninitialized ID
as XEN_VCPU_ID_INVALID (which is U32_MAX).

On the destination host, attempting to restore this state fails because
kvm_xen_vcpu_set_attr() explicitly rejects values >= KVM_MAX_VCPUS:

arch/x86/kvm/xen.c:kvm_xen_vcpu_set_attr() {
    ...
    case KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID:
        if (data->u.vcpu_id >= KVM_MAX_VCPUS)
            r = -EINVAL;
        else {
    ...
}

Will this cause KVM to unconditionally reject the valid state returned by
GET_ATTR and return -EINVAL during live migration?

>  	vcpu->arch.xen.poll_evtchn = 0;
>  
>  	timer_setup(&vcpu->arch.xen.poll_timer, cancel_evtchn_poll, 0);

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

Re: [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The vCPU index is not unwound back to -1 on the `create_vcpu_fd()` error path, defeating the patch's attempt to suppress false-positive lockdep assertions for unreachable vCPUs.
--

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

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

This commit initializes a vCPU's index to -1 during creation to prevent KVM
from incorrectly treating an unreachable vCPU as vCPU0. It also unwinds the
index back to -1 if inserting the vCPU into the array fails, suppressing
false positive lockdep assertions during teardown.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e44c20c049610..98da4c889ffc6 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
[ ... ]
> @@ -4216,11 +4218,18 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>  		goto unlock_vcpu_destroy;
>  	}
>  
> +	/*
> +	 * Set the vCPU's index *before* the vCPU is reachable by other tasks.
> +	 * Unwind the index back to -1 on failure so that KVM can use the index
> +	 * to detect that the vCPU is unreachable, e.g. for lockdep asserts.
> +	 */
>  	vcpu->vcpu_idx = atomic_read(&kvm->online_vcpus);
>  	r = xa_insert(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, GFP_KERNEL_ACCOUNT);
>  	WARN_ON_ONCE(r == -EBUSY);
> -	if (r)
> +	if (r) {
> +		vcpu->vcpu_idx = -1;
>  		goto unlock_vcpu_destroy;
> +	}
>  
>  	/*
>  	 * Now it's all set up, let userspace reach it.  Grab the vCPU's mutex

[Severity: Medium]
Should the vCPU index also be unwound back to -1 on the create_vcpu_fd()
error path?

If a local process exhausts its file descriptor limit, create_vcpu_fd()
will fail and jump to the kvm_put_xa_erase label in kvm_vm_ioctl_create_vcpu():

kvm_put_xa_erase:
	mutex_unlock(&vcpu->mutex);
	kvm_put_kvm_no_destroy(kvm);
	xa_erase(&kvm->vcpu_array, vcpu->vcpu_idx);
unlock_vcpu_destroy:
	mutex_unlock(&kvm->lock);

Since xa_erase() is called here without resetting vcpu->vcpu_idx = -1, does this
leave the vCPU with a valid index during the subsequent kvm_arch_vcpu_destroy()
call, potentially triggering the same lockdep assertion splats this patch
is trying to prevent?

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

Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

[Sean Christopherson]

On Thu, Jun 25, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> > --- a/arch/x86/kvm/xen.c
> > +++ b/arch/x86/kvm/xen.c
> > @@ -1614,13 +1614,13 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
> >  	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> >  		return false;
> >  
> > +	if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> > +		*r = -EINVAL;
> > +		return true;
> > +	}
> > +
> 
> [Severity: High]
> Does hoisting this check unintentionally reject cross-vCPU commands that should
> be delegated to userspace?

Gah, I misread the "default" case.

Re: [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created

[Sean Christopherson]

On Thu, Jun 25, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index e44c20c049610..98da4c889ffc6 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> [ ... ]
> > @@ -4216,11 +4218,18 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
> >  		goto unlock_vcpu_destroy;
> >  	}
> >  
> > +	/*
> > +	 * Set the vCPU's index *before* the vCPU is reachable by other tasks.
> > +	 * Unwind the index back to -1 on failure so that KVM can use the index
> > +	 * to detect that the vCPU is unreachable, e.g. for lockdep asserts.
> > +	 */
> >  	vcpu->vcpu_idx = atomic_read(&kvm->online_vcpus);
> >  	r = xa_insert(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, GFP_KERNEL_ACCOUNT);
> >  	WARN_ON_ONCE(r == -EBUSY);
> > -	if (r)
> > +	if (r) {
> > +		vcpu->vcpu_idx = -1;
> >  		goto unlock_vcpu_destroy;
> > +	}
> >  
> >  	/*
> >  	 * Now it's all set up, let userspace reach it.  Grab the vCPU's mutex
> 
> [Severity: Medium]
> Should the vCPU index also be unwound back to -1 on the create_vcpu_fd()
> error path?

What the?  Oh, ugh.  I thought we had eliminated erasing a vCPU from the xarray,
but what we eliminated was unwinding online_vcpus.  

[syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv

[syzbot ci]

syzbot ci has tested the following series

[v3] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv
https://lore.kernel.org/all/20260625223623.3376478-1-seanjc@google.com
* [PATCH v3 01/10] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller
* [PATCH v3 02/10] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo()
* [PATCH v3 03/10] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses
* [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
* [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
* [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created
* [PATCH v3 07/10] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper
* [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid
* [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()
* [PATCH v3 10/10] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses

and found the following issue:
WARNING in kvm_hv_vcpu_uninit

Full report is available here:
https://ci.syzbot.org/series/6864cbe8-ce00-47d1-b4b1-abc7d1528d4e

***

WARNING in kvm_hv_vcpu_uninit

tree:      kvm-next
URL:       https://kernel.googlesource.com/pub/scm/virt/kvm/kvm/
base:      a204badd8432f93b7e862e7dac6db0fe3d65f370
arch:      amd64
compiler:  Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config:    https://ci.syzbot.org/builds/0627391d-c4d6-4820-99f0-31a8b51ba783/config
syz repro: https://ci.syzbot.org/findings/cdadfdf2-8445-4193-a3a6-bf878176b493/syz_repro

------------[ cut here ]------------
debug_locks && !(lock_is_held(&(&vcpu->mutex)->dep_map) || vcpu->vcpu_idx < 0 || !refcount_read(&vcpu->kvm->users_count))
WARNING: ./include/linux/kvm_host.h:996 at kvm_lockdep_assert_vcpu_is_locked_or_unreachable include/linux/kvm_host.h:994 [inline], CPU#1: syz.1.34/5967
WARNING: ./include/linux/kvm_host.h:996 at to_hv_vcpu arch/x86/kvm/hyperv.h:79 [inline], CPU#1: syz.1.34/5967
WARNING: ./include/linux/kvm_host.h:996 at kvm_hv_vcpu_uninit+0x198/0x210 arch/x86/kvm/hyperv.c:906, CPU#1: syz.1.34/5967
Modules linked in:
CPU: 1 UID: 0 PID: 5967 Comm: syz.1.34 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:kvm_lockdep_assert_vcpu_is_locked_or_unreachable include/linux/kvm_host.h:994 [inline]
RIP: 0010:to_hv_vcpu arch/x86/kvm/hyperv.h:79 [inline]
RIP: 0010:kvm_hv_vcpu_uninit+0x198/0x210 arch/x86/kvm/hyperv.c:906
Code: 48 89 df e8 0a 55 d8 00 48 c7 03 00 00 00 00 eb 05 e8 5c 16 6d 00 5b 41 5c 41 5e 41 5f 5d e9 ff 16 4d 0a cc e8 49 16 6d 00 90 <0f> 0b 90 e9 65 ff ff ff 48 c7 c1 e0 bb 2f 90 80 e1 07 80 c1 03 38
RSP: 0018:ffffc9000390f960 EFLAGS: 00010293
RAX: ffffffff8158e9e7 RBX: ffff8881bc192880 RCX: ffff88816ac70000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 0000000000000001 R08: ffff888101fa1743 R09: 1ffff110203f42e8
R10: dffffc0000000000 R11: ffffed10203f42e9 R12: 0000000000000000
R13: 00000000fffffff8 R14: ffff888101fa1740 R15: dffffc0000000000
FS:  00007f47c1e886c0(0000) GS:ffff8882a92b6000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000200000c64000 CR3: 000000016a5e7000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 kvm_arch_vcpu_destroy+0x1a9/0x380 arch/x86/kvm/x86.c:9413
 kvm_vm_ioctl_create_vcpu+0x615/0x990 virt/kvm/kvm_main.c:4269
 kvm_vm_ioctl+0x886/0xd30 virt/kvm/kvm_main.c:5168
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:597 [inline]
 __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:583
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f47c0f9caeb
Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
RSP: 002b:00007f47c1e87ee0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007f47c1215fa0 RCX: 00007f47c0f9caeb
RDX: 0000000000000000 RSI: 000000000000ae41 RDI: 00000000000000f7
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000200000c00000 R11: 0000000000000246 R12: 0000200000c00000
R13: 0000000000000153 R14: 0000200000000400 R15: 00007ffe526558c8
 </TASK>


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).

The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.

Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set

[David Woodhouse]

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

On Thu, 2026-06-25 at 15:36 -0700, Sean Christopherson wrote:
> Explicitly invalidate KVM's internal Xen vCPU ID during vCPU creation
> instead of *trying* to set the Xen ID to the vCPU index by default, and
> forward singleshot timer hypercalls to userspace if the VMM hasn't set the
> Xen ID via KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.  Using the vCPU's index as its
> default Xen ID is reasonable in concept, but in practice is horribly flawed
> as the index is left as '0' until after vCPU initialization completes, i.e.
> every vCPU gets a Xen ID of '0' by default.
> 
> Forward hypercalls to userspace instead of trying to salvage any kind of
> default behavior, as all userspace implementations that support multiple
> vCPUs either don't enable the timer, are guaranteed to set Xen ID, or work
> only because *all* guests also screw up the singleshot timer hypercalls.
> The last scenarios is extremely unlikely given that Linux-as-a-guest uses
> the actual Xen vCPU ID when making timer hypercalls.  In other words, for
> all intents and purposes, KVM's ABI is already that userspace must set the
> Xen vCPU ID, so just commit to that ABI.
> 
> Note, KVM's handling of KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID restricts the ID to
> KVM_MAX_VCPUS, so there's no chance of a valid ID colliding with -1u.

Nit: XEN_VCPU_ID_INVALID isn't -1u; it's U32_MAX.

I'd have been slightly happier if you'd used an explicit -1U, perhaps
with a name of its own (although I guess we wouldn't want to call it
KVM_VCPU_IDX_INVALID in the *Xen* part).

But this works.

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

I'd have put this *after* your patch 5 though (qv).

> Link: https://lore.kernel.org/all/20260612233017.1F9771F000E9@smtp.kernel.org
> Suggested-by: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/xen.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 694b31c1fcc9..27c1aeeab8af 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -22,6 +22,7 @@
>  #include <xen/interface/version.h>
>  #include <xen/interface/event_channel.h>
>  #include <xen/interface/sched.h>
> +#include <xen/xen-ops.h>
>  
>  #include <asm/xen/cpuid.h>
>  #include <asm/pvclock.h>
> @@ -1610,6 +1611,9 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  	if (!kvm_xen_timer_enabled(vcpu))
>  		return false;
>  
> +	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> +		return false;
> +
>  	switch (cmd) {
>  	case VCPUOP_set_singleshot_timer:
>  		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> @@ -2299,7 +2303,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
>  
>  void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
>  {
> -	vcpu->arch.xen.vcpu_id = vcpu->vcpu_idx;
> +	vcpu->arch.xen.vcpu_id = XEN_VCPU_ID_INVALID;
>  	vcpu->arch.xen.poll_evtchn = 0;
>  
>  	timer_setup(&vcpu->arch.xen.poll_timer, cancel_evtchn_poll, 0);

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

Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

[David Woodhouse]

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

On Thu, 2026-06-25 at 15:36 -0700, Sean Christopherson wrote:
> Hoist the checks on the Xen vCPU ID when handling set_singleshot_timer and
> stop_singleshot_timer hypercalls out of their individual case-statements,
> so that both checks on the ID are in common code.  kvm_xen_hcall_vcpu_op()
> is already doubly committed to handling only singleshot timer hypercalls,
> and even if that were to change in the future, the function could simply
> be renamed and turned into a helper specifically for timer hypercalls.
> 
> No functional change intended.

Makes sense. In fact these hypercalls are the *only* VCPUOP_xxx calls
for which Xen has that restriction (otherwise it would be pointless to
have the vcpu argument at all). Which is why we did the check in the
individual cases.

But these are *also* the only VCPUOP calls we're ever likely to
accelerate in the kernel, so that's actually fine. I would prefer to
see a comment above the check though.

And let's move your patch 4 to come *after* this semantic change.

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/xen.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 27c1aeeab8af..db10f12d10cf 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1614,13 +1614,13 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
>  		return false;
>  

	/*
	 * Only the set/stop singleshot timer calls are accelerated
	 * by KVM, and they both operate only on the same vCPU.
	 */
> +	if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> +		*r = -EINVAL;
> +		return true;
> +	}
> +
>  	switch (cmd) {
>  	case VCPUOP_set_singleshot_timer:
> -		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> -			*r = -EINVAL;
> -			return true;
> -		}
> -
>  		/*
>  		 * The only difference for 32-bit compat is the 4 bytes of
>  		 * padding after the interesting part of the structure. So
> @@ -1648,10 +1648,6 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  		return true;
>  
>  	case VCPUOP_stop_singleshot_timer:
> -		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> -			*r = -EINVAL;
> -			return true;
> -		}
>  		kvm_xen_stop_timer(vcpu);
>  		*r = 0;
>  		return true;

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