* [PATCH v3 01/10] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 02/10] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo() Sean Christopherson
` (9 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 02/10] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo()
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 01/10] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 03/10] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
` (8 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 03/10] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 01/10] KVM: x86/hyperv: Get target FIFO in hv_tlb_flush_enqueue(), not caller Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 02/10] KVM: x86/hyperv: Check for NULL vCPU Hyper-V object in kvm_hv_get_tlb_flush_fifo() Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set Sean Christopherson
` (7 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (2 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 03/10] KVM: x86/hyperv: Ensure vCPU's Hyper-V object is initialized on cross-vCPU accesses Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:50 ` sashiko-bot
2026-06-26 8:05 ` David Woodhouse
2026-06-25 22:36 ` [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls Sean Christopherson
` (6 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
2026-06-25 22:36 ` [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set Sean Christopherson
@ 2026-06-25 22:50 ` sashiko-bot
2026-06-26 8:05 ` David Woodhouse
1 sibling, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-06-25 22:50 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
2026-06-25 22:36 ` [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set Sean Christopherson
2026-06-25 22:50 ` sashiko-bot
@ 2026-06-26 8:05 ` David Woodhouse
2026-06-26 14:27 ` Sean Christopherson
1 sibling, 1 reply; 31+ messages in thread
From: David Woodhouse @ 2026-06-26 8:05 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov, Paolo Bonzini,
Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
2026-06-26 8:05 ` David Woodhouse
@ 2026-06-26 14:27 ` Sean Christopherson
2026-06-26 15:19 ` David Woodhouse
0 siblings, 1 reply; 31+ messages in thread
From: Sean Christopherson @ 2026-06-26 14:27 UTC (permalink / raw)
To: David Woodhouse
Cc: Vitaly Kuznetsov, Paolo Bonzini, Paul Durrant, kvm, linux-kernel,
syzbot+5b32c49cd8f005e65654, syzbot+5d2b94b77112148d1744
On Fri, Jun 26, 2026, David Woodhouse wrote:
> 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).
Ya, I was 50/50 on having KVM define its own INVALID macro versus reusing what
the Xen-the-guest has. I didn't want to open code the literal, because that
would require open coding a somewhat magical value in multiple locations. And
having XEN_VCPU_ID_INVALID and KVM_XEN_VCPU_ID_INVALID, with the same value,
seemed even worse than reusing a macro that isn't strictly owned by KVM.
What if I also add a compile-timer assertion? In the end, KVM doesn't care what
value XEN_VCPU_ID_INVALID uses, so long as it can't colled with what is allowed
by KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.
diff --git arch/x86/kvm/xen.c arch/x86/kvm/xen.c
index 3ed6686e0a1a..5c60ed9ef2cc 100644
--- arch/x86/kvm/xen.c
+++ arch/x86/kvm/xen.c
@@ -1103,6 +1103,8 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
break;
case KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID:
+ BUILD_BUG_ON(XEN_VCPU_ID_INVALID < KVM_MAX_VCPUS);
+
if (data->u.vcpu_id >= KVM_MAX_VCPUS)
r = -EINVAL;
else {
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set
2026-06-26 14:27 ` Sean Christopherson
@ 2026-06-26 15:19 ` David Woodhouse
0 siblings, 0 replies; 31+ messages in thread
From: David Woodhouse @ 2026-06-26 15:19 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, Paolo Bonzini, Paul Durrant, kvm, linux-kernel,
syzbot+5b32c49cd8f005e65654, syzbot+5d2b94b77112148d1744
[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]
On Fri, 2026-06-26 at 07:27 -0700, Sean Christopherson wrote:
>
> > > 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).
>
> Ya, I was 50/50 on having KVM define its own INVALID macro versus reusing what
> the Xen-the-guest has. I didn't want to open code the literal, because that
> would require open coding a somewhat magical value in multiple locations. And
> having XEN_VCPU_ID_INVALID and KVM_XEN_VCPU_ID_INVALID, with the same value,
> seemed even worse than reusing a macro that isn't strictly owned by KVM.
>
> What if I also add a compile-timer assertion? In the end, KVM doesn't care what
> value XEN_VCPU_ID_INVALID uses, so long as it can't colled with what is allowed
> by KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID.
Yeah, that makes sense. It's consistent with the build-time assertions
we have elsewhere in the Xen code to document other such things we hold
to be true.
I'd probably have put it along with the code which *uses*
XEN_VCPU_ID_INVALID but I guess it doesn't matter.
> diff --git arch/x86/kvm/xen.c arch/x86/kvm/xen.c
> index 3ed6686e0a1a..5c60ed9ef2cc 100644
> --- arch/x86/kvm/xen.c
> +++ arch/x86/kvm/xen.c
> @@ -1103,6 +1103,8 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> break;
>
> case KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID:
> + BUILD_BUG_ON(XEN_VCPU_ID_INVALID < KVM_MAX_VCPUS);
> +
> if (data->u.vcpu_id >= KVM_MAX_VCPUS)
> r = -EINVAL;
> else {
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (3 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 04/10] KVM: x86/xen: Punt singleshot timer hcalls to userspace if Xen vCPU ID isn't set Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:43 ` sashiko-bot
2026-06-26 8:11 ` David Woodhouse
2026-06-25 22:36 ` [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
` (5 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-25 22:36 ` [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls Sean Christopherson
@ 2026-06-25 22:43 ` sashiko-bot
2026-06-25 23:30 ` Sean Christopherson
2026-06-26 8:11 ` David Woodhouse
1 sibling, 1 reply; 31+ messages in thread
From: sashiko-bot @ 2026-06-25 22:43 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-25 22:43 ` sashiko-bot
@ 2026-06-25 23:30 ` Sean Christopherson
0 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 23:30 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-25 22:36 ` [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls Sean Christopherson
2026-06-25 22:43 ` sashiko-bot
@ 2026-06-26 8:11 ` David Woodhouse
2026-06-26 14:19 ` Sean Christopherson
1 sibling, 1 reply; 31+ messages in thread
From: David Woodhouse @ 2026-06-26 8:11 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov, Paolo Bonzini,
Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-26 8:11 ` David Woodhouse
@ 2026-06-26 14:19 ` Sean Christopherson
2026-06-26 15:32 ` David Woodhouse
0 siblings, 1 reply; 31+ messages in thread
From: Sean Christopherson @ 2026-06-26 14:19 UTC (permalink / raw)
To: David Woodhouse
Cc: Vitaly Kuznetsov, Paolo Bonzini, Paul Durrant, kvm, linux-kernel,
syzbot+5b32c49cd8f005e65654, syzbot+5d2b94b77112148d1744
On Fri, Jun 26, 2026, David Woodhouse wrote:
> 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.
Sashiko pointed out that the patch is broken as-is, because the effective
"default" case will reject hypercalls if the vcpu_id doesn't match instead of
routing those to userspace. The easiest way to deal with that is to pull the
cmd check out of the switch-statement, e.g.
struct vcpu_set_singleshot_timer oneshot;
struct x86_exception e;
if (cmd != VCPUOP_set_singleshot_timer &&
cmd != VCPUOP_stop_singleshot_timer)
return false;
if (!kvm_xen_timer_enabled(vcpu))
return false;
if (vcpu->arch.xen.vcpu_id != vcpu_id) {
*r = -EINVAL;
return true;
}
if (cmd == VCPUOP_set_singleshot_timer) {
/*
* The only difference for 32-bit compat is the 4 bytes of
* padding after the interesting part of the structure. So
* for a faithful emulation of Xen we have to *try* to copy
* the padding and return -EFAULT if we can't. Otherwise we
* might as well just have copied the 12-byte 32-bit struct.
*/
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
offsetof(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
sizeof_field(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, flags) !=
offsetof(struct vcpu_set_singleshot_timer, flags));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, flags) !=
sizeof_field(struct vcpu_set_singleshot_timer, flags));
if (kvm_read_guest_virt(vcpu, param, &oneshot, longmode ? sizeof(oneshot) :
sizeof(struct compat_vcpu_set_singleshot_timer), &e)) {
*r = -EFAULT;
return true;
}
kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, false);
} else {
kvm_xen_stop_timer(vcpu);
}
*r = 0;
return true;
> 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.
This?
/*
* Reject the hypercall if the guest is trying to start/stop the timer
* for a different vCPU. Xen per-vCPU hypercalls take a target vCPU as
* a common parameter, as all per-vCPU hypercalls *except* single-shot
* timer updates can be cross-vCPU.
*/
if (vcpu->arch.xen.vcpu_id != vcpu_id) {
*r = -EINVAL;
return true;
}
> And let's move your patch 4 to come *after* this semantic change.
Agreed.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-26 14:19 ` Sean Christopherson
@ 2026-06-26 15:32 ` David Woodhouse
2026-06-26 18:12 ` Sean Christopherson
0 siblings, 1 reply; 31+ messages in thread
From: David Woodhouse @ 2026-06-26 15:32 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, Paolo Bonzini, Paul Durrant, kvm, linux-kernel,
syzbot+5b32c49cd8f005e65654, syzbot+5d2b94b77112148d1744
[-- Attachment #1: Type: text/plain, Size: 3577 bytes --]
On Fri, 2026-06-26 at 07:19 -0700, Sean Christopherson wrote:
> On Fri, Jun 26, 2026, David Woodhouse wrote:
> > On Thu, 2026-06-25 at 15:36 -0700, Sean Christopherson wrote:
> > > Hoist the checks on the Xe
+ if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> > > + return false;n 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.
>
> Sashiko pointed out that the patch is broken as-is, because the effective
> "default" case will reject hypercalls if the vcpu_id doesn't match instead of
> routing those to userspace. The easiest way to deal with that is to pull the
> cmd check out of the switch-statement, e.g.
>
> struct vcpu_set_singleshot_timer oneshot;
> struct x86_exception e;
>
> if (cmd != VCPUOP_set_singleshot_timer &&
> cmd != VCPUOP_stop_singleshot_timer)
> return false;
>
> if (!kvm_xen_timer_enabled(vcpu))
> return false;
>
You dropped the
+ if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
+ return false;
part. Although that's kind of fair since I did ask you to put this
patch first, before that.
But really, while this rearrangement you propose would work... this
patch was intended as a cosmetic cleanup, and this seems less pretty
than what we had before.
How about keeping your patch 4 which just returns false for invalid
vcpu_id, and dropping patch 5 completely? Or if you really want to
tackle it, start with this...
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1745,8 +1745,10 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
params[1], &r);
break;
case __HYPERVISOR_vcpu_op:
- handled = kvm_xen_hcall_vcpu_op(vcpu, longmode, params[0], params[1],
- params[2], &r);
+ if (params[0] == VCPUOP_set_singleshot_timer ||
+ params[0] == VCPUOP_stop_singleshot_timer)
+ handled = kvm_xen_hcall_vcpu_op(vcpu, longmode, params[0], params[1],
+ params[2], &r);
break;
case __HYPERVISOR_set_timer_op: {
u64 timeout = params[0];
>
> > 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.
>
> This?
>
> /*
> * Reject the hypercall if the guest is trying to start/stop the timer
> * for a different vCPU. Xen per-vCPU hypercalls take a target vCPU as
> * a common parameter, as all per-vCPU hypercalls *except* single-shot
> * timer updates can be cross-vCPU.
> */
> if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> *r = -EINVAL;
> return true;
> }
I guess, although I'm no longer keen on the consolidation at all.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls
2026-06-26 15:32 ` David Woodhouse
@ 2026-06-26 18:12 ` Sean Christopherson
0 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-26 18:12 UTC (permalink / raw)
To: David Woodhouse
Cc: Vitaly Kuznetsov, Paolo Bonzini, Paul Durrant, kvm, linux-kernel,
syzbot+5b32c49cd8f005e65654, syzbot+5d2b94b77112148d1744
On Fri, Jun 26, 2026, David Woodhouse wrote:
> On Fri, 2026-06-26 at 07:19 -0700, Sean Christopherson wrote:
> > On Fri, Jun 26, 2026, David Woodhouse wrote:
> > > On Thu, 2026-06-25 at 15:36 -0700, Sean Christopherson wrote:
> > > > Hoist the checks on the Xe
>
> + if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> > > > + return false;n 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.
> >
> > Sashiko pointed out that the patch is broken as-is, because the effective
> > "default" case will reject hypercalls if the vcpu_id doesn't match instead of
> > routing those to userspace. The easiest way to deal with that is to pull the
> > cmd check out of the switch-statement, e.g.
> >
> > struct vcpu_set_singleshot_timer oneshot;
> > struct x86_exception e;
> >
> > if (cmd != VCPUOP_set_singleshot_timer &&
> > cmd != VCPUOP_stop_singleshot_timer)
> > return false;
> >
> > if (!kvm_xen_timer_enabled(vcpu))
> > return false;
> >
>
> You dropped the
>
> + if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
> + return false;
>
> part. Although that's kind of fair since I did ask you to put this
> patch first, before that.
Heh, it's still there, I just didn't include it in this snippet.
> But really, while this rearrangement you propose would work... this
> patch was intended as a cosmetic cleanup, and this seems less pretty
> than what we had before.
I disagree. The old code uses a common check for kvm_xen_timer_enabled(), which
is confusing and actively dangerous, as evidenced by my goof, because it's easy
to miss that the only reason KVM can bail early for !kvm_xen_timer_enabled() is
because it returns false. Punting hypercalls that are never accelerated by KVM
because some other hypercall happens to be disabled is nasty.
> How about keeping your patch 4 which just returns false for invalid
> vcpu_id, and dropping patch 5 completely? Or if you really want to
> tackle it, start with this...
>
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1745,8 +1745,10 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
> params[1], &r);
> break;
> case __HYPERVISOR_vcpu_op:
> - handled = kvm_xen_hcall_vcpu_op(vcpu, longmode, params[0], params[1],
> - params[2], &r);
> + if (params[0] == VCPUOP_set_singleshot_timer ||
> + params[0] == VCPUOP_stop_singleshot_timer)
> + handled = kvm_xen_hcall_vcpu_op(vcpu, longmode, params[0], params[1],
> + params[2], &r);
But then kvm_xen_hcall_vcpu_op() is a misleading name, and this also splits the
checks on @cmd. E.g. even with this:
if (params[0] != VCPUOP_set_singleshot_timer &&
params[0] != VCPUOP_stop_singleshot_timer)
break;
handled = kvm_xen_hcall_single_shot_timer(vcpu, longmode, params[0],
params[1], params[2], &r);
break;
Then kvm_xen_hcall_single_shot_timer() either looks flawed if it does:
if (cmd == VCPUOP_set_singleshot_timer) {
...
kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, false);
} else {
kvm_xen_stop_timer(vcpu);
}
because it doesn't explicitly check that cmd is set_singleshot_timer or
stop_singleshot_timer. We could do:
if (cmd == VCPUOP_set_singleshot_timer) {
...
kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, false);
} else {
WARN_ON_ONCE(cmd != VCPUOP_stop_singleshot_timer);
kvm_xen_stop_timer(vcpu);
}
but that's rather ridiculous given that there's exactly one path to this code.
IMO, this:
struct vcpu_set_singleshot_timer oneshot;
struct x86_exception e;
if (cmd != VCPUOP_set_singleshot_timer &&
cmd != VCPUOP_stop_singleshot_timer)
return false;
if (!kvm_xen_timer_enabled(vcpu))
return false;
if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
return false;
/*
* Reject the hypercall if the guest is trying to start/stop the timer
* for a different vCPU. Xen per-vCPU hypercalls take a target vCPU as
* a common parameter, as all per-vCPU hypercalls *except* single-shot
* timer updates can be cross-vCPU.
*/
if (vcpu->arch.xen.vcpu_id != vcpu_id) {
*r = -EINVAL;
return true;
}
if (cmd == VCPUOP_set_singleshot_timer) {
/*
* The only difference for 32-bit compat is the 4 bytes of
* padding after the interesting part of the structure. So
* for a faithful emulation of Xen we have to *try* to copy
* the padding and return -EFAULT if we can't. Otherwise we
* might as well just have copied the 12-byte 32-bit struct.
*/
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
offsetof(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
sizeof_field(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, flags) !=
offsetof(struct vcpu_set_singleshot_timer, flags));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, flags) !=
sizeof_field(struct vcpu_set_singleshot_timer, flags));
if (kvm_read_guest_virt(vcpu, param, &oneshot, longmode ? sizeof(oneshot) :
sizeof(struct compat_vcpu_set_singleshot_timer), &e)) {
*r = -EFAULT;
return true;
}
kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, false);
} else {
kvm_xen_stop_timer(vcpu);
}
*r = 0;
return true;
is logically more sound and easier to read than what we currently have:
struct vcpu_set_singleshot_timer oneshot;
struct x86_exception e;
if (!kvm_xen_timer_enabled(vcpu))
return false;
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
* for a faithful emulation of Xen we have to *try* to copy
* the padding and return -EFAULT if we can't. Otherwise we
* might as well just have copied the 12-byte 32-bit struct.
*/
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
offsetof(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
sizeof_field(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, flags) !=
offsetof(struct vcpu_set_singleshot_timer, flags));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, flags) !=
sizeof_field(struct vcpu_set_singleshot_timer, flags));
if (kvm_read_guest_virt(vcpu, param, &oneshot, longmode ? sizeof(oneshot) :
sizeof(struct compat_vcpu_set_singleshot_timer), &e)) {
*r = -EFAULT;
return true;
}
kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, false);
*r = 0;
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;
}
return false;
especially once the "vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID" check comes
along, because while this technically works, it's even more confusing because
vcpu->arch.xen.vcpu_id isn't necessarily check for the other ops, e.g. if @vcpu_id
targets a different vCPU.
struct vcpu_set_singleshot_timer oneshot;
struct x86_exception e;
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) {
*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
* for a faithful emulation of Xen we have to *try* to copy
* the padding and return -EFAULT if we can't. Otherwise we
* might as well just have copied the 12-byte 32-bit struct.
*/
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
offsetof(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, timeout_abs_ns) !=
sizeof_field(struct vcpu_set_singleshot_timer, timeout_abs_ns));
BUILD_BUG_ON(offsetof(struct compat_vcpu_set_singleshot_timer, flags) !=
offsetof(struct vcpu_set_singleshot_timer, flags));
BUILD_BUG_ON(sizeof_field(struct compat_vcpu_set_singleshot_timer, flags) !=
sizeof_field(struct vcpu_set_singleshot_timer, flags));
if (kvm_read_guest_virt(vcpu, param, &oneshot, longmode ? sizeof(oneshot) :
sizeof(struct compat_vcpu_set_singleshot_timer), &e)) {
*r = -EFAULT;
return true;
}
kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, false);
*r = 0;
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;
}
return false;
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (4 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:57 ` sashiko-bot
2026-06-25 22:36 ` [PATCH v3 07/10] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper Sean Christopherson
` (4 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created
2026-06-25 22:36 ` [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
@ 2026-06-25 22:57 ` sashiko-bot
2026-06-25 23:31 ` Sean Christopherson
0 siblings, 1 reply; 31+ messages in thread
From: sashiko-bot @ 2026-06-25 22:57 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created
2026-06-25 22:57 ` sashiko-bot
@ 2026-06-25 23:31 ` Sean Christopherson
0 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 23:31 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 07/10] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (5 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:36 ` [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
` (3 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (6 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 07/10] KVM: Move nVMX's lockdep logic for vcpu->mutex to a common helper Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:50 ` sashiko-bot
2026-06-25 22:36 ` [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid
2026-06-25 22:36 ` [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
@ 2026-06-25 22:50 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-06-25 22:50 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (7 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 08/10] KVM: x86: Treat a vCPU as unreachable if its index is invalid Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-25 22:50 ` sashiko-bot
2026-06-25 22:36 ` [PATCH v3 10/10] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses Sean Christopherson
2026-06-26 7:06 ` [syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv syzbot ci
10 siblings, 1 reply; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu()
2026-06-25 22:36 ` [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
@ 2026-06-25 22:50 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-06-25 22:50 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 10/10] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (8 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 09/10] KVM: x86/hyperv: Assert vCPU's mutex is held in to_hv_vcpu() Sean Christopherson
@ 2026-06-25 22:36 ` Sean Christopherson
2026-06-26 7:06 ` [syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv syzbot ci
10 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-25 22:36 UTC (permalink / raw)
To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini,
David Woodhouse, Paul Durrant
Cc: kvm, linux-kernel, syzbot+5b32c49cd8f005e65654,
syzbot+5d2b94b77112148d1744
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv
2026-06-25 22:36 [PATCH v3 00/10] KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv Sean Christopherson
` (9 preceding siblings ...)
2026-06-25 22:36 ` [PATCH v3 10/10] KVM: x86/hyperv: Use {READ,WRITE}_ONCE for cross-task synic->active accesses Sean Christopherson
@ 2026-06-26 7:06 ` syzbot ci
2026-06-26 13:24 ` Sean Christopherson
10 siblings, 1 reply; 31+ messages in thread
From: syzbot ci @ 2026-06-26 7:06 UTC (permalink / raw)
To: dwmw2, kvm, linux-kernel, paul, pbonzini, seanjc, syzbot,
vkuznets
Cc: syzbot, syzkaller-bugs
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.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv
2026-06-26 7:06 ` [syzbot ci] Re: KVM: x86/hyperv: Fix racy usage of vcpu->arch.hyperv syzbot ci
@ 2026-06-26 13:24 ` Sean Christopherson
0 siblings, 0 replies; 31+ messages in thread
From: Sean Christopherson @ 2026-06-26 13:24 UTC (permalink / raw)
To: syzbot ci
Cc: dwmw2, kvm, linux-kernel, paul, pbonzini, syzbot, vkuznets,
syzbot, syzkaller-bugs
On Fri, Jun 26, 2026, syzbot ci wrote:
> ------------[ 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
This is the same thing Sashiko pointed out; I completely botched the cleanup if
vCPU creation fails.
^ permalink raw reply [flat|nested] 31+ messages in thread