Re: [PATCH 1/3] KVM: arm64: nv: Print nested mmu info in kvm_ptdump_guest_show()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing Use-After-Free and NULL pointer dereference in debugfs handlers due to concurrent MMU teardown.
--

commit 09a8b839f22b6806feaaf12ccca901b340acd24f
Author: Wei-Lin Chang <weilin.chang@arm.com>

KVM: arm64: nv: Print nested mmu info in kvm_ptdump_guest_show()

This commit extends kvm_ptdump_guest_show() to print VTCR, VTTBR, and s2
enabled information for valid nested MMUs. It also converts the MMU lock usage
to use a lock guard.

> diff --git a/arch/arm64/kvm/ptdump.c b/arch/arm64/kvm/ptdump.c
> index c9140e22abcf1..2a6afe67646af 100644
> --- a/arch/arm64/kvm/ptdump.c
> +++ b/arch/arm64/kvm/ptdump.c
> @@ -155,9 +155,16 @@ static int kvm_ptdump_guest_show(struct seq_file *m, void *unused)
>  		.seq		= m,
>  	};
>  
> -	write_lock(&kvm->mmu_lock);
> +	guard(write_lock)(&kvm->mmu_lock);
> +	if (kvm_is_nested_s2_mmu(kvm, mmu)) {
> +		if (!kvm_s2_mmu_valid(mmu)) {
> +			seq_puts(m, "invalid nested mmu\n");
> +			return 0;
> +		}
> +		seq_printf(m, "0x%016llx 0x%016llx %d\n", mmu->tlb_vttbr,
> +			   mmu->tlb_vtcr, mmu->nested_stage2_enabled ? 1 : 0);
> +	}
>  	ret = kvm_pgtable_walk(mmu->pgt, 0, BIT(mmu->pgt->ia_bits), &walker);

[Severity: High]
This is a pre-existing issue, but can this dereference of mmu->pgt cause a
NULL pointer dereference or a Use-After-Free?

If the process holding the VM dies, kvm_mmu_notifier_release() is triggered.
This uninitializes the stage-2 MMU and sets mmu->pgt = NULL for the canonical
MMU. However, the KVM struct can be kept alive by open file descriptors for
debugfs files, so kvm_destroy_vm() is not called yet.

If a concurrent reader accesses the stage2_page_tables debugfs file and
invokes kvm_ptdump_guest_show(), could it blindly evaluate
BIT(mmu->pgt->ia_bits) here and trigger a NULL pointer dereference?

Similarly, in kvm_pgtable_debugfs_open() for other debugfs files like range
or levels, the mmu->pgt pointer is cached at open time without synchronization:

	pgtable = mmu->pgt;

	ret = single_open(file, show, pgtable);

Could this lead to a Use-After-Free in the show handlers if the page tables
are freed by concurrent MMU teardown before a read occurs?

> -	write_unlock(&kvm->mmu_lock);
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623142443.648972-1-weilin.chang@arm.com?part=1