Re: [PATCH v3 3/3] KVM: riscv: Register ptdump with debugfs on guest creation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Lockless traversal of `kvm->arch.pgd` using host `mmap_write_lock` instead of `kvm->mmu_lock` leads to a Use-After-Free if gstage page tables are concurrently freed.
- [High] The generic host page table walker is structurally incompatible with KVM guest stage-2 page tables, leading to bogus walks and kernel panics.
- [Medium] Passing a NULL `kvm->arch.pgd` to the core page table walker causes it to silently fall back to walking the host process's page tables.
- [Medium] Mutating the static global array `gstage_pg_levels` without synchronization causes data races and permanently corrupts the output layout across different VMs.
- [Medium] Incompatible reuse of the host's `note_page()` function breaks range coalescing and prints incorrect output names.
- [Low] Linker error when KVM is compiled as a loadable module due to missing export of `ptdump_walk_pgd`.
--

--- Patch [3]: [PATCH v3 3/3] KVM: riscv: Register ptdump with debugfs on guest creation ---
commit 5cf74731d352654675c1f47276baa2185271e44d
Author: Dylan.Wu <fredwudi0305@gmail.com>

KVM: riscv: Register ptdump with debugfs on guest creation

Implement the gstage page table dumper for RISC-V KVM, which exposes
the guest's gstage page tables via debugfs. When
CONFIG_PTDUMP_GSTAGE_DEBUGFS is enabled, a "gstage_page_tables" file
will be created under each VM's debugfs directory.

> diff --git a/arch/riscv/kvm/ptdump.c b/arch/riscv/kvm/ptdump.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/riscv/kvm/ptdump.c
[ ... ]
> +static void gstage_note_page(struct ptdump_state *pt_st, unsigned long addr,
> +			     int level, u64 val)
> +{
> +	struct ptdump_pg_state *st = container_of(pt_st, struct ptdump_pg_state, ptdump);
> +
> +	note_page(pt_st, addr, level, val);
> +
> +	if (level >= 0)
> +		st->current_prot = val & gstage_pg_levels[level].mask;
> +}

[Severity: Medium]
Will reusing the host note_page() function break range coalescing and
produce incorrect output?

The host note_page() function hardcodes the use of kernel_pg_levels to
compute the page protection mask. Because kernel_pg_levels and
gstage_pg_levels mask different bits, won't note_page() falsely detect a
protection change for almost every page before we override it here?

Additionally, it seems note_page() will use kernel_pg_levels to print out
the directory names, potentially printing incorrect level names for the
guest entries.

[ ... ]
> +static int gstage_ptdump_show(struct seq_file *m, void *v)
> +{
> +	struct kvm *kvm = m->private;
[ ... ]
> +	for (i = 0; i < ARRAY_SIZE(gstage_pg_levels); i++) {
> +		gstage_pg_levels[i].mask = 0;
> +		for (j = 0; j < ARRAY_SIZE(gstage_pte_bits); j++)
> +			gstage_pg_levels[i].mask |= gstage_pte_bits[j].mask;
> +	}
> +
> +	if (kvm->arch.pgd_levels < 5)
> +		gstage_pg_levels[1].name = "PGD";
> +	if (kvm->arch.pgd_levels < 4)
> +		gstage_pg_levels[2].name = "PGD";
> +	if (kvm->arch.pgd_levels < 3)
> +		gstage_pg_levels[3].name = "PGD";

[Severity: Medium]
Does mutating the static global gstage_pg_levels array here cause data
races?

Since debugfs files can be read concurrently for multiple VMs with different
pgd_levels, won't these lockless writes corrupt the array for subsequent
reads of other VMs?

> +
> +	ptdump_walk_pgd(&st.ptdump, kvm->mm, kvm->arch.pgd);

[Severity: Critical]
Does this lockless traversal risk a use-after-free of the gstage page tables?

KVM guest stage-2 page tables are strictly managed under kvm->mmu_lock. The
debugfs walker acquires the host process's mmap_write_lock via
ptdump_walk_pgd(), but does not hold kvm->mmu_lock.

If a concurrent KVM operation such as a memslot removal or an MMU notifier
fires and calls kvm_riscv_gstage_unmap_range(), it will free the intermediate
page table pages. Will this walker concurrently read freed memory?

[Severity: High]
Is the generic host page table walker structurally compatible with KVM guest
stage-2 page tables?

The generic walker relies on compile-time host macros like PGDIR_SHIFT and
PTRS_PER_PGD. However, RISC-V KVM gstage page tables have dynamic geometries
based on kvm->arch.pgd_levels.

When the host macros are applied to the gstage root, will the walker
compute incorrect indices and treat guest leaf PTEs as intermediate
directories, causing out-of-bounds memory accesses?

[Severity: Medium]
What happens if the debugfs file is read before the gstage root is
allocated?

At VM initialization, kvm->arch.pgd is NULL. If a NULL pgd is passed to
walk_pgd_range(), won't it silently fall back to walking the host process's
page tables via pgd_offset(walk->mm, addr)?

[Severity: Low]
Will this cause a build failure when KVM is compiled as a loadable module?

The core kernel symbol ptdump_walk_pgd() is not exported to modules via
EXPORT_SYMBOL(), which might cause a linker error here when KVM is built as
a module.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728161449.190058-1-fredwudi0305@gmail.com?part=3