Re: [PATCH] KVM: arm64: Take the SRCU lock for page table walks in fault injection and AT emulation

[Oliver Upton <oupton@kernel.org>]

Hi Hyunwoo,

On Wed, Jun 03, 2026 at 01:04:20AM +0900, Hyunwoo Kim wrote:
> inject_abt64() rewalks the guest stage-1 page tables via
> __kvm_find_s1_desc_level() when injecting an abort for a failed S1PTW, and
> __kvm_at_s12() calls kvm_walk_nested_s2() to perform the stage-2
> translation. Both walks reference kvm->memslots through kvm_read_guest(),
> which reads the descriptors, and __kvm_at_swap_desc(), which updates the
> access flag, so they must run while holding the kvm->srcu read lock.
> __kvm_at_swap_desc() asserts srcu_read_lock_held() on entry, and the other
> callers of these walks, handle_at_slow(), kvm_translate_vncr() and
> kvm_handle_guest_abort(), take the lock before calling them.
> 
> inject_abt64() is reached from the SEA and size fault injection paths,
> which run before kvm_handle_guest_abort() takes the lock, and
> __kvm_at_s12() does not hold the lock across the stage-2 walk. Take the
> kvm->srcu read lock with guard(srcu) in both places so that it is held for
> the duration of the walk.

Just state the expectation that srcu is held rather than giving the
play by play. Perhaps:

  walk_s1() and kvm_walk_nested_s2() expect to be called while holding
  kvm->srcu to guard against memslot changes. While this is generally
  the case, __kvm_at_s12() and __kvm_find_s1_desc_level() call into the
  respective walkers without taking kvm->srcu.

  Fix by acquiring kvm->srcu prior to the table walk in both instances.

> Cc: stable@vger.kernel.org
> Fixes: 50f77dc87f13 ("KVM: arm64: Populate level on S1PTW SEA injection")
> Fixes: be04cebf3e78 ("KVM: arm64: nv: Add emulation of AT S12E{0,1}{R,W}")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>

I'd prefer if we scoped the critical section to only the relevant calls
to the software table walk, like below.

-- 
Thanks,
Oliver

diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 9f8f0ae8e86e..889c2c15d7bd 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -1569,7 +1569,8 @@ int __kvm_at_s12(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 	/* Do the stage-2 translation */
 	ipa = (par & GENMASK_ULL(47, 12)) | (vaddr & GENMASK_ULL(11, 0));
 	out.esr = 0;
-	ret = kvm_walk_nested_s2(vcpu, ipa, &out);
+	scoped_guard(srcu, &vcpu->kvm->srcu)
+		ret = kvm_walk_nested_s2(vcpu, ipa, &out);
 	if (ret < 0)
 		return ret;
 
@@ -1665,7 +1666,8 @@ int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa, int *level)
 	}
 
 	/* Walk the guest's PT, looking for a match along the way */
-	ret = walk_s1(vcpu, &wi, &wr, va);
+	scoped_guard(srcu, &vcpu->kvm->srcu)
+		ret = walk_s1(vcpu, &wi, &wr, va);
 	switch (ret) {
 	case -EINTR:
 		/* We interrupted the walk on a match, return the level */