Re: [PATCH v3 1/2] KVM: arm64: ptdump: Check the page tables aren't freed when accessing

[Vincent Donnefort <vdonnefort@google.com>]

On Thu, Sep 17, 2026 at 08:30:49PM +0100, Wei-Lin Chang wrote:
> On Thu, Sep 17, 2026 at 09:11:50AM +0100, Vincent Donnefort wrote:
> > On Thu, Sep 17, 2026 at 12:03:36AM +0100, Wei-Lin Chang wrote:
> > > An open debugfs file keeps the KVM structure alive, but does not prevent
> > > mmu notifier release from freeing the stage-2 page tables when the VMM’s
> > > address space is torn down. Therefore page tables belonging to the mmus
> > > could have been freed when a thread opens or reads the ptdump files.
> > > Take the mmu_lock and check mmu->pgt is still alive before accessing the
> > > page tables.
> > 
> > As Sashiko said, the read_lock is probably enough, including the existing one in
> > kvm_ptdump_guest_show()
> > 
> > With that change:
> > 
> > Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
> > Tested-by: Vincent Donnefort <vdonnefort@google.com>
> 
> Thanks for the review and testing!
> 
> I agree taking read_lock is enough for most of these, but changing
> kvm_ptdump_guest_show() to read_lock could result in a dump showing
> weird output e.g. 0-sized ranges. This happens when the dump reads a
> block, and a parallel fault turns that block into a table, and the dump
> descends into the table later.

Now you say it, I remember discussing that with Sebastian when he wrote the
patches.

> 
> This is debugfs afterall so I think it isn't a dealbreaker, but it adds
> another purpose to this patch. Maybe we can change
> kvm_ptdump_guest_show() into taking a read_lock when someone reports a
> scalability problem when dumping the page tables.
> 
> I'll stick to changing the other ones into taking the read_lock now.
> 
> For future reference: KVM_PGTABLE_WALK_SHARED is required if we want to
> change kvm_ptdump_guest_show() into taking a read_lock.
> 
> Thanks,
> Wei-Lin Chang
> 
> > 

-- 
Vincent