Re: [PATCH 2/2] KVM: arm64: nv: Fix null ptr deref in kvm_nested_s2_unmap() on S2 teardown

[Marc Zyngier <maz@kernel.org>]

On Wed, 12 Aug 2026 14:31:21 +0100,
"Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> 
> Commit 7270cc9157f4 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU
> notifiers") introduced VNCR_EL2 invalidation in kvm_nested_s2_unmap().
> 
> However at the point of this being performed concurrent stage 2 teardown of
> a nested guest can cause kvm->arch.mmu.pgt to be set to NULL.
> 
> This happens in kvm_flush_shadow_all() -> kvm_arch_flush_shadow_all() ->
> kvm_free_stage2_pgd() and is performed under the kvm->mmu_lock.
> 
> Commit ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page
> tables") introduced the teardown of the entire nested MMU range, which then
> invokes stage2_apply_range() with resched=true:
> 
> mmu_notifier_invalidate_range_start()
>   -> ... -> kvm_mmu_notifier_invalidate_range_start()
>     -> kvm_mmu_unmap_gfn_range()
>       -> kvm_unmap_gfn_range()
>         -> kvm_nested_s2_unmap()
>           -> kvm_stage2_unmap_range()
>             -> __unmap_stage2_range()
>                 -> stage2_apply_range()
> 
> This means that stage2_apply_range() can drop the kvm->mmu_lock and thus
> concurrent progress can be made in lockstep with
> kvm_arch_flush_shadow_all().
> 
> If kvm_arch_flush_shadow_all() advances ahead of stage2_apply_range() and
> completes its operation it guarantees a NULL pointer deref.
> 
> Since kvm_free_stage2_pgd() is performed under the kvm->mmu_lock this will
> either be observed NULL or not and serialised against
> kvm_free_stage2_pgd().
> 
> Resolve the issue by explicitly checking for a NULL value. Since this
> shouldn't be possible if blocking is not allowed, raise a warning if it is
> ever NULL in this case.
> 
> Fixes: 7270cc9157f4 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU notifiers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  arch/arm64/kvm/nested.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 17123f0b6dab..9fc0b1696dcb 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1295,7 +1295,11 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
>  			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
>  	}
>  
> -	kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
> +	/* NULL pgt should only be possible if raced when mmu_lock dropped. */
> +	if (kvm->arch.mmu.pgt)
> +		kvm_invalidate_vncr_ipa(kvm, 0, BIT(kvm->arch.mmu.pgt->ia_bits));
> +	else
> +		WARN_ON(!may_block);

I don't think the WARN_ON() should be here. We already have one at the
iterator level, which will fire in the same conditions.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.