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

> 
> Also for the ipa_range file and stage2_levels file, switch from keeping
> the pointer to the page tables to keeping the pointer to the mmu.
> 
> Fixes: 7c4f73548ed1 ("KVM: arm64: Register ptdump with debugfs on guest creation")
> Reported-by: Sashiko AI <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/kvmarm/20260623144054.B91D21F000E9@smtp.kernel.org/
> Signed-off-by: Wei-Lin Chang <weilin.chang@arm.com>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/kvm/ptdump.c | 43 +++++++++++++++++++++++++++++++----------
>  1 file changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/ptdump.c b/arch/arm64/kvm/ptdump.c
> index 69899797dbad..4ead856c181a 100644
> --- a/arch/arm64/kvm/ptdump.c
> +++ b/arch/arm64/kvm/ptdump.c
> @@ -115,15 +115,28 @@ static int kvm_ptdump_build_levels(struct ptdump_pg_level *level, u32 start_lvl)
>  
>  static struct kvm_ptdump_guest_state *kvm_ptdump_parser_create(struct kvm_s2_mmu *mmu)
>  {
> +	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
>  	struct kvm_ptdump_guest_state *st;
> -	struct kvm_pgtable *pgtable = mmu->pgt;
> +	s8 start_level;
>  	int ret;
>  
> +	/*
> +	 * We only need the pgt start level to initialize the ptdump, get it
> +	 * while holding the mmu_lock. It's fine if the pgt gets freed
> +	 * afterwards, we'll check again when doing the actual dump.
> +	 */
> +	scoped_guard(write_lock, &kvm->mmu_lock) {
> +		if (mmu->pgt)
> +			start_level = mmu->pgt->start_level;
> +		else
> +			return ERR_PTR(-ENOENT);
> +	}
> +
>  	st = kzalloc_obj(struct kvm_ptdump_guest_state, GFP_KERNEL_ACCOUNT);
>  	if (!st)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = kvm_ptdump_build_levels(&st->level[0], pgtable->start_level);
> +	ret = kvm_ptdump_build_levels(&st->level[0], start_level);
>  	if (ret) {
>  		kfree(st);
>  		return ERR_PTR(ret);
> @@ -149,6 +162,9 @@ static int kvm_ptdump_guest_show(struct seq_file *m, void *unused)
>  	};
>  
>  	guard(write_lock)(&kvm->mmu_lock);
> +	if (!mmu->pgt)
> +		return 0;
> +
>  	st->parser_state = (struct ptdump_pg_state) {
>  		.marker		= &st->ipa_marker[0],
>  		.end_address	= BIT(mmu->pgt->ia_bits),
> @@ -211,17 +227,27 @@ static const struct file_operations kvm_ptdump_guest_fops = {
>  
>  static int kvm_pgtable_range_show(struct seq_file *m, void *unused)
>  {
> -	struct kvm_pgtable *pgtable = m->private;
> +	struct kvm_s2_mmu *mmu = m->private;
> +	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> +
> +	guard(write_lock)(&kvm->mmu_lock);
> +
> +	if (mmu->pgt)
> +		seq_printf(m, "%2u\n", mmu->pgt->ia_bits);
>  
> -	seq_printf(m, "%2u\n", pgtable->ia_bits);
>  	return 0;
>  }
>  
>  static int kvm_pgtable_levels_show(struct seq_file *m, void *unused)
>  {
> -	struct kvm_pgtable *pgtable = m->private;
> +	struct kvm_s2_mmu *mmu = m->private;
> +	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> +
> +	guard(write_lock)(&kvm->mmu_lock);
> +
> +	if (mmu->pgt)
> +		seq_printf(m, "%1d\n", KVM_PGTABLE_MAX_LEVELS - mmu->pgt->start_level);
>  
> -	seq_printf(m, "%1d\n", KVM_PGTABLE_MAX_LEVELS - pgtable->start_level);
>  	return 0;
>  }
>  
> @@ -230,15 +256,12 @@ static int kvm_pgtable_debugfs_open(struct inode *m, struct file *file,
>  {
>  	struct kvm_s2_mmu *mmu = m->i_private;
>  	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> -	struct kvm_pgtable *pgtable;
>  	int ret;
>  
>  	if (!kvm_get_kvm_safe(kvm))
>  		return -ENOENT;
>  
> -	pgtable = mmu->pgt;
> -
> -	ret = single_open(file, show, pgtable);
> +	ret = single_open(file, show, mmu);
>  	if (ret < 0)
>  		kvm_put_kvm(kvm);
>  	return ret;
> -- 
> 2.43.0
> 

-- 
Vincent