Re: [PATCH v2 1/4] RISC-V: KVM: Add the split page cache for ioctl context

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Destruction of stage-2 PGD during `mmu_notifier_release` allows vCPUs to execute with BARE mode stage-2 translation, granting guests full access to host physical memory
- [High] NULL pointer dereference when enabling dirty logging after MMU destruction
--

commit 5935831ace6c3305e677dc574f5aeeda39b0da2a
Author: Wang Yechao <wang.yechao255@zte.com.cn>

RISC-V: KVM: Add the split page cache for ioctl context

Add the split page cache for dirty logging enablement and the
KVM_CLEAR_DIRTY_LOG ioctl.

[ ... ]

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 2d3def024270c..0676937bd9a1c 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c

[ ... ]

> @@ -572,6 +573,8 @@ void kvm_riscv_mmu_free_pgd(struct kvm *kvm)
>  
>  	if (pgd)
>  		free_pages((unsigned long)pgd, get_order(kvm_riscv_gstage_pgd_size));
> +
> +	kvm_mmu_free_memory_cache(&kvm->arch.pgd_split_page_cache);
>  }

[Severity: High]
This is a pre-existing issue, but can
kvm_arch_mmu_enable_log_dirty_pt_masked() trigger a NULL pointer dereference
after the PGD is freed here?

When a process exits, mmu_notifier_release() invokes kvm_riscv_mmu_free_pgd(),
which frees the stage-2 PGD and sets kvm->arch.pgd = NULL and
kvm->arch.pgd_levels = 0.

However, if the KVM VM file descriptor is still held open by another process
or thread, a subsequent KVM_CLEAR_DIRTY_LOG ioctl would call
kvm_arch_mmu_enable_log_dirty_pt_masked() in arch/riscv/kvm/mmu.c.

This unconditionally initializes the gstage and accesses the PGD via
kvm_riscv_gstage_get_leaf(). Since gstage->pgd_levels is 0, current_level
wraps around to UINT_MAX, and it dereferences gstage->pgd (which is NULL)
plus an out-of-bounds index.

>  
>  void kvm_riscv_mmu_update_hgatp(struct kvm_vcpu *vcpu)

[Severity: Critical]
This is a pre-existing issue, but does freeing the stage-2 PGD during
mmu_notifier_release allow vCPUs to execute with BARE mode stage-2
translation?

If the VM file descriptor is kept open after kvm_riscv_mmu_free_pgd() sets
kvm->arch.pgd_phys = 0 and kvm->arch.pgd_levels = 0, another process can still
invoke KVM_RUN.

When the vCPU resumes, kvm_riscv_mmu_update_hgatp() in arch/riscv/kvm/mmu.c
programs the hgatp CSR using kvm_riscv_gstage_mode(0), which evaluates to
BARE mode:

arch/riscv/kvm/mmu.c:kvm_riscv_mmu_update_hgatp() {
	...
	unsigned long hgatp = kvm_riscv_gstage_mode(ka->pgd_levels)
			      << HGATP_MODE_SHIFT;

	hgatp |= (READ_ONCE(ka->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
	hgatp |= (ka->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;

	ncsr_write(CSR_HGATP, hgatp);
	...
}

With BARE mode stage-2 translation, the hardware treats guest physical
addresses as host physical addresses. Could this grant guests full access to
host physical memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603175256408L0jnqGs1cJGc0ijCdujci@zte.com.cn?part=1