[PATCH v2 10/11] KVM: x86/xen: Take kvm->srcu in __kvm_xen_has_interrupt()

0 views
Skip to first unread message

David Woodhouse

unread,
5:48 AM (7 hours ago) 5:48 AM
to sea...@google.com, pbon...@redhat.com, dw...@amazon.co.uk, pa...@xen.org, joao.m....@oracle.com, boris.o...@oracle.com, ankur....@oracle.com, tg...@kernel.org, mi...@redhat.com, b...@alien8.de, dave....@linux.intel.com, h...@zytor.com, x...@kernel.org, syzbot+208f7f...@syzkaller.appspotmail.com, syzkall...@googlegroups.com, suryasai...@gmail.com, l...@intel.com, k...@vger.kernel.org, linux-...@vger.kernel.org
From: David Woodhouse <dw...@amazon.co.uk>

kvm_gpc_check() checks the cached memslot generation against the current
one, which dereferences kvm->memslots and therefore requires kvm->srcu to
be held. __kvm_xen_has_interrupt() does not take it.

Most callers do happen to hold kvm->srcu, but not all of them:

- kvm_emulate_halt() on the VM-Exit path, via kvm_vcpu_has_events() and
kvm_cpu_has_extint(). vcpu_enter_guest() drops the vCPU's SRCU lock
before entering the guest, so it is not held on the way back out.

- kvm_vcpu_block() -> kvm_vcpu_check_block() -> kvm_arch_vcpu_runnable(),
which is the case the existing comment in this function describes.

On a PROVE_RCU kernel the former produces:

WARNING: suspicious RCU usage
include/linux/kvm_host.h:1092 suspicious rcu_dereference_check() usage!
...
kvm_gpc_check+0x344/0x3e0 [kvm]
__kvm_xen_has_interrupt+0x83/0x310 [kvm]
kvm_cpu_has_extint+0x1ff/0x370 [kvm]
kvm_cpu_has_interrupt+0x16/0x100 [kvm]
kvm_vcpu_has_events+0x4ce/0x690 [kvm]
kvm_emulate_halt+0x52/0x1f0 [kvm]
vmx_vcpu_run+0x988/0x2630 [kvm_intel]

Use guard(srcu) so that the three existing early returns don't each need
an explicit unlock. SRCU read sections nest, so this is harmless on the
paths which already hold it, and srcu_read_lock() does not sleep, so it
is also safe in the atomic case which this function already handles.

Fixes: 7caf9571563e ("KVM: x86/xen: Use gfn_to_pfn_cache for vcpu_info")
Cc: sta...@vger.kernel.org
Signed-off-by: David Woodhouse <dw...@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/xen.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 99f3ffa64fdb..ff55ff290afb 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -716,6 +716,16 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
BUILD_BUG_ON(sizeof(rc) !=
sizeof_field(struct compat_vcpu_info, evtchn_upcall_pending));

+ /*
+ * kvm_gpc_check() checks the memslot generation, so kvm->srcu must be
+ * held. Most callers hold it already, but this is also reached from
+ * kvm_emulate_halt() on the VM-Exit path and from kvm_vcpu_block(),
+ * where vcpu_enter_guest() has already dropped the vCPU's SRCU lock.
+ * Taking SRCU does not sleep, so it is safe even in the atomic case
+ * which is handled below.
+ */
+ guard(srcu)(&v->kvm->srcu);
+
read_lock_irqsave(&gpc->lock, flags);
while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
read_unlock_irqrestore(&gpc->lock, flags);
--
2.55.0

Reply all
Reply to author
Forward
0 new messages