[PATCH v2] KVM: x86: Take SRCU in kvm_zap_gfn_range()

Chengfeng Ye <[email protected]>
Newsgroups org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
kvm_zap_gfn_range() walks memslots and rmaps and may drop mmu_lock to
reschedule. Callers that do not already hold kvm->srcu (or slots_lock)
can race with memslot deletion: synchronize_srcu_expedited() does not
wait, kvm_free_memslot() frees the old slot and its rmap, and the zap
resumes on freed memory.

The VFIO noncoherent-DMA path hits this by zapping the entire GPA
space without SRCU. KASAN reported:

  BUG: KASAN: vmalloc-out-of-bounds in slot_rmap_walk_next+0x82/0x1c0
  Read of size 8 at addr ffffc900005c1008
  Call Trace:
   slot_rmap_walk_next+0x82/0x1c0
   __kvm_rmap_zap_gfn_range+0x17a/0x280
   kvm_zap_gfn_range+0x2a6/0x6a0
   kvm_vfio_set_attr+0x576/0x770
   kvm_device_ioctl+0x1ff/0x3b0

Take SRCU inside kvm_zap_gfn_range() so every caller is covered.
Nesting with an existing kvm->srcu critical section is fine; the
helper uses a local index. Drop the now-redundant SRCU pair from
__kvm_set_or_clear_apicv_inhibit().

Fixes: 362ff6dca541 ("KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops")
Cc: [email protected]
Signed-off-by: Chengfeng Ye <[email protected]>
---
v2:
- Take SRCU inside kvm_zap_gfn_range() with guard(srcu)(), as suggested
  by Sean Christopherson, instead of wrapping only the noncoherent-DMA
  caller.
- Drop the now-redundant SRCU pair from
  __kvm_set_or_clear_apicv_inhibit(). Nested kvm->srcu is fine; only
  kvm_vcpu_srcu_read_lock() cannot nest with itself.

 arch/x86/kvm/mmu/mmu.c | 2 ++
 arch/x86/kvm/x86.c     | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index a61750f8e1e3..ae55a77e5a05 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -7047,6 +7047,8 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
 	if (WARN_ON_ONCE(gfn_end <= gfn_start))
 		return;
 
+	guard(srcu)(&kvm->srcu);
+
 	write_lock(&kvm->mmu_lock);
 
 	kvm_mmu_invalidate_start(kvm);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 69469bbdc84a..adccd4a8e6c1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10986,10 +10986,8 @@ void __kvm_set_or_clear_apicv_inhibit(struct kvm *kvm,
 		kvm->arch.apicv_inhibit_reasons = new;
 		if (new) {
 			unsigned long gfn = gpa_to_gfn(APIC_DEFAULT_PHYS_BASE);
-			int idx = srcu_read_lock(&kvm->srcu);
 
 			kvm_zap_gfn_range(kvm, gfn, gfn+1);
-			srcu_read_unlock(&kvm->srcu, idx);
 		}
 	} else {
 		kvm->arch.apicv_inhibit_reasons = new;
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.