Re: [PATCH v5 13/18] KVM: arm64: Alloc pkvm_hyp_vm using pKVM heap allocator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Memcg charge bypass and potential charge leak due to `pkvm_hyp_vm` allocation transitioning to the global EL2 heap without proper host return or memcg accounting.
--

commit 9d9a73aa6d8fa15436c6b53dd36960b9dade0e30
Author: Vincent Donnefort <vdonnefort@google.com>

KVM: arm64: Alloc pkvm_hyp_vm using pKVM heap allocator

Transition the allocation of the hypervisor VM state structure (pkvm_hyp_vm)
from the host to the hypervisor using the new pKVM heap allocator. The host
now uses pkvm_call_hyp_req() to invoke __pkvm_init_vm, which automatically
handles any top-up requests if the hypervisor runs out of heap memory.

> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 0a041d41b9ea4..cabcfb59a89d2 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
[ ... ]
> @@ -847,15 +846,15 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,
>  	vm_size = pkvm_get_hyp_vm_size(nr_vcpus);
>  	pgd_size = kvm_pgtable_stage2_pgd_size(host_mmu.arch.mmu.vtcr);
>  
> -	ret = -ENOMEM;
> -
> -	hyp_vm = map_donated_memory(vm_hva, vm_size);
> -	if (!hyp_vm)
> -		goto err_remove_mappings;
> +	hyp_vm = hyp_alloc(vm_size);

[Severity: High]
Is it safe to allocate the VM struct from the global EL2 allocator here?

This draws from top-up pages that lack memcg constraints, whereas the
previous implementation donated memory that was explicitly allocated with
GFP_KERNEL_ACCOUNT by the host.

> +	if (!hyp_vm) {
> +		ret = hyp_alloc_errno();
> +		goto err_unpin_kvm;
> +	}
[ ... ]
> @@ -1053,8 +1052,7 @@ int __pkvm_finalize_teardown_vm(pkvm_handle_t handle)
>  		teardown_donated_memory(mc, hyp_vcpu, sizeof(*hyp_vcpu));
>  	}
>  
> -	vm_size = pkvm_get_hyp_vm_size(hyp_vm->kvm.created_vcpus);
> -	teardown_donated_memory(mc, hyp_vm, vm_size);
> +	hyp_free(hyp_vm);

[Severity: High]
Does freeing the VM struct back into the global EL2 allocator rather than
returning it directly to the host create a charge stranding issue if memcg
accounting is re-enabled?

If accounting is restored at the top-up layer, any memcg charges applied
during top-up would be stranded indefinitely in the EL2 global pool until
reclaimed by the host shrinker. This would permanently leave the charge in
the original memcg even when those pages are subsequently reused by a
different VM.

>  	hyp_unpin_shared_mem(host_kvm, host_kvm + 1);
>  	return 0;
>  
> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> index ea96744f41fb0..8505965dbfb21 100644
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c
[ ... ]
> @@ -230,28 +230,15 @@ static int __pkvm_create_hyp_vm(struct kvm *kvm)
>  	if (!pgd)
>  		return -ENOMEM;
>  
> -	/* Allocate memory to donate to hyp for vm and vcpu pointers. */
> -	hyp_vm_sz = PAGE_ALIGN(size_add(PKVM_HYP_VM_SIZE,
> -					size_mul(sizeof(void *),
> -						 kvm->created_vcpus)));
> -	hyp_vm = alloc_pages_exact(hyp_vm_sz, GFP_KERNEL_ACCOUNT);
> -	if (!hyp_vm) {
> -		ret = -ENOMEM;
> -		goto free_pgd;
> -	}
> -
> -	/* Donate the VM memory to hyp and let hyp initialize it. */
> -	ret = kvm_call_hyp_nvhe(__pkvm_init_vm, kvm, hyp_vm, pgd);
> +	ret = pkvm_call_hyp_req(__pkvm_init_vm, kvm, pgd);

[Severity: High]
Does this change bypass memcg accounting for the pkvm_hyp_vm allocation?

Previously, this memory was allocated with GFP_KERNEL_ACCOUNT. By delegating
this allocation to the EL2 allocator via pkvm_call_hyp_req(), the memory is
provisioned through un-accounted top-ups.

When pkvm_call_hyp_req() triggers a PKVM_HYP_REQ_HYP_ALLOC to the host, it
is serviced by pkvm_hyp_topup(). The top-up memcache is initialized with
zeroed flags (omitting HYP_MEMCACHE_ACCOUNT_KMEMCG), which causes
hyp_mc_alloc_fn() to use raw GFP_KERNEL. 

Because pkvm_hyp_vm scales with nr_vcpus and is no longer charged to the
creating task's memcg, can this allow an unprivileged container to bypass
its memory limits and exhaust host memory, potentially leading to a host
OOM?

>  	if (ret)
> -		goto free_vm;
> +		goto free_pgd;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901080941.997769-1-vdonnefort@google.com?part=13