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

[Hyunwoo Kim <imv4bel@gmail.com>]

On Tue, Jun 02, 2026 at 11:44:58PM -0700, Oliver Upton wrote:
> 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 for the review.

I agree this direction is the better patch. I'll do some more testing 
and then submit a v2.

> 
> -- 
> 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 */


Best regards,
Hyunwoo Kim