Re: [PATCH v3 1/3] KVM: riscv: Implement KVM_PRE_FAULT_MEMORY

[sashiko-bot@kernel.org]

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

New issues:
- [High] The KVM_PRE_FAULT_MEMORY ioctl handler loops infinitely in the kernel if it encounters a HWPOISON page and the userspace thread has blocked SIGBUS.
- [Medium] kvm_arch_vcpu_pre_fault_memory() can silently skip requested pages and report false progress when mapping a split THP page.

Pre-existing issues:
- [High] Infinite VMExit page fault loop when a guest accesses an unaligned THP address on a split page table.
- [High] Physical page table leak when overwriting a lower-level page table directory with a huge page leaf PTE.
--

--- Patch [1]: [PATCH v3 1/3] KVM: riscv: Implement KVM_PRE_FAULT_MEMORY ---
commit ac8ca417e02ba68eb7b7c58fbc637ee417dc2c72
Author: Jinyu Tang <jinyu.tang@linux.dev>

KVM: riscv: Implement KVM_PRE_FAULT_MEMORY

This commit introduces the KVM_PRE_FAULT_MEMORY ioctl handler for RISC-V.
It reuses the existing G-stage fault mapping path with a read access to
populate KVM page tables before running a vCPU, looping until a mapping
is installed or an exit condition is met.

> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index b0474fcf065aa..ebd1e8a4b9e38 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
> @@ -280,6 +280,9 @@ int kvm_riscv_gstage_map_page(...)
>  		} else if (ALIGN_DOWN(PFN_PHYS(pte_pfn(ptep_get(ptep))), page_size) == hpa) {
>  			kvm_riscv_gstage_update_pte_prot(gstage, ptep_level, gpa, ptep, prot);
> +			out_map->addr = ALIGN_DOWN(gpa, page_size);
> +			out_map->level = ptep_level;
> +			out_map->pte = ptep_get(ptep);
>  			return 0;
>  		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does this lead to an
infinite VMExit loop when a guest accesses an unaligned THP address on
a split page table?

During fault handling, transparent_hugepage_adjust() in kvm_riscv_mmu_map()
can silently overwrite the original faulting gpa with the 2MB-aligned
base address.

If the G-stage page table is currently split into 4KB PTEs, this block in
kvm_riscv_gstage_map_page() intentionally updates only the 4KB PTE at the
newly aligned gpa. The originally faulting 4KB page remains unmapped,
causing the guest to immediately fault again upon resumption.

[ ... ]

>  	out_map->pte = pfn_pte(PFN_DOWN(hpa), prot);
>  	out_map->pte = pte_mkdirty(out_map->pte);
>  
>  	return kvm_riscv_gstage_set_pte(gstage, pcache, out_map);
>  }

[Severity: High]
This is a pre-existing issue, but does this leak physical page tables when
overwriting a lower-level page table directory with a huge page leaf PTE?

If 4KB leaf PTEs inside a THP-backed region are zapped (for example, via
MADV_DONTNEED) but the page directory remains, a subsequent fault triggers
a THP mapping. kvm_riscv_gstage_get_leaf() fails to find a leaf.

The code then falls through here to kvm_riscv_gstage_set_pte(), which blindly
overwrites the existing directory entry with a huge page leaf PTE. Because
freeing page tables is not yet supported, the orphaned child page tables
appear to be leaked.

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 8a0aa5e0e216e..c71c1ebb07857 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c

[ ... ]

> +	for (;;) {
> +		if (signal_pending(current))
> +			return -EINTR;
> +
> +		if (kvm_check_request(KVM_REQ_VM_DEAD, vcpu))
> +			return -EIO;
> +
> +		cond_resched();
> +		ret = kvm_riscv_mmu_map(vcpu, memslot, range->gpa, hva, false, &out_map);
> +		if (ret)
> +			return ret;
> +
> +		if (pte_val(out_map.pte))
> +			break;
> +	}

[Severity: High]
Can this loop spin infinitely if the userspace thread blocks SIGBUS and
KVM encounters a hardware-poisoned page?

Looking at kvm_riscv_mmu_map(), it handles HWPOISON by calling
send_sig_mceerr() and returning 0 without populating out_map.

If the userspace thread has SIGBUS blocked, send_sig_mceerr() queues the
signal but might not set TIF_SIGPENDING. In that scenario, signal_pending()
won't catch the pending signal, out_map.pte remains empty, and the loop
will retry infinitely without yielding back to userspace.

> +
> +	map_size = PAGE_SIZE << (out_map.level * kvm_riscv_gstage_index_bits);
> +	end = ALIGN_DOWN(range->gpa, map_size) + map_size;
> +
> +	return min_t(u64, range->size, end - range->gpa);
> +}

[Severity: Medium]
Does this miscalculate the mapped progress when handling a split THP page?

If transparent_hugepage_adjust() aligns the gpa down to the 2MB base and
kvm_riscv_gstage_map_page() falls back to a 4KB mapping, the 4KB PTE is
populated at the base address. out_map.addr is correctly set to this base.

However, the progress here is calculated using the original unaligned
range->gpa instead of out_map.addr. If the requested GPA was at an offset,
KVM might falsely report progress for the unmapped requested page while
actually mapping the unrequested base page.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814114507.583424-1-jinyu.tang@linux.dev?part=1