Re: [PATCH 08/20] KVM: arm64: Add a range to __pkvm_host_reclaim_page_guest()

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

Pre-existing issues:
- [High] The guest teardown reclaim path relies on guest stage-2 page-table walks to validate and translate the GFN for ownership transition instead of enumerating by EL2 ownership metadata. If a guest page lacks a valid stage-2 PTE, the reclaim fails and the memory is permanently leaked.
--

--- Patch [8]: [PATCH 08/20] KVM: arm64: Add a range to __pkvm_host_reclaim_page_guest() ---
commit 268a05000f92ceab485eb6ee82a421828dd4d08d
Author: Vincent Donnefort <[email protected]>

KVM: arm64: Add a range to __pkvm_host_reclaim_page_guest()

This patch adds a nr_pages argument to the __pkvm_host_reclaim_page_guest()
hypercall to prepare for stage-2 huge mappings support for protected VMs. It
allows reclaiming either a single page or a block of pages.

> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index 9ea448895c15e..ce637f1a55b37 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c

[ ... ]

> @@ -1365,35 +1386,40 @@ int __pkvm_host_force_reclaim_page_guest(phys_addr_t phys)
>  	return ret;
>  }
>  
> -int __pkvm_host_reclaim_page_guest(u64 gfn, struct pkvm_hyp_vm *vm)
> +int __pkvm_host_reclaim_page_guest(u64 gfn, u64 nr_pages, struct pkvm_hyp_vm *vm)
>  {
>  	u64 ipa = hyp_pfn_to_phys(gfn);
>  	kvm_pte_t pte;
>  	u64 phys;
> +	u64 size;
>  	int ret;
>  
> +	ret = __guest_check_transition_size(0, ipa, nr_pages, &size);
> +	if (ret)
> +		return ret;
> +
>  	host_lock_component();
>  	guest_lock_component(vm);
>  
> -	ret = get_valid_guest_pte(vm, ipa, &pte, &phys);
> +	ret = get_valid_guest_pte(vm, ipa, size, &pte, &phys);
>  	if (ret)
>  		goto unlock;

[Severity: High]
This is a pre-existing issue, but does relying on a guest stage-2 page-table
walk here to find the page to reclaim lead to a physical memory leak?

If a guest page lacks a valid stage-2 PTE (for instance, if a previous
mapping failed or the guest unmapped it), get_valid_guest_pte() returns
-ENOENT.

This causes the function to exit early, skipping the ownership transition and
leaving the physical memory permanently marked as PKVM_PAGE_OWNED by the dead
guest, which can lead to host resource exhaustion.

>  	switch (guest_get_page_state(pte, ipa)) {
>  	case PKVM_PAGE_OWNED:

[ ... ]

> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> index 58d8474b563e6..a9645480d1648 100644
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c

[ ... ]

> @@ -373,7 +373,8 @@ static int __pkvm_pgtable_stage2_reclaim(struct kvm_pgtable *pgt, u64 start, u64
>  		struct page *page;
>  
>  		ret = kvm_call_hyp_nvhe(__pkvm_reclaim_dying_guest_page,
> -					handle, mapping->gfn);
> +					handle, mapping->gfn,
> +					mapping->nr_pages);
>  		if (WARN_ON(ret))
>  			continue;

[Severity: High]
This is a pre-existing issue, but does driving the reclaim from the host's
local tracking tree here contribute to the leak mentioned above?

Because this loop enumerates pages using the host's mapping tree instead
of having EL2 enumerate them by their recorded ownership metadata, any
discrepancy where a page is owned but lacks a valid stage-2 PTE will
result in the memory permanently leaking when the hypercall fails.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8