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

[Lorenzo Stoakes (ARM)]

On Sat, Aug 22, 2026 at 08:32:04AM +0100, Marc Zyngier wrote:
> On Wed, 12 Aug 2026 14:31:21 +0100,
> "Lorenzo Stoakes (ARM)" <ljs@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> > ---
> >  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.

Ack, was an abundance of caution, will take out on respin.

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

--
Cheers, Lorenzo