Re: [PATCH v4 14/17] KVM: arm64: Alloc pkvm_hyp_vcpu using pKVM heap allocator

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The dynamic allocation of the hypervisor vCPU and VM state structures via the pKVM heap allocator bypasses memory cgroup (kmemcg) accounting, allowing an unprivileged userspace process to bypass memcg limits.
--

--- Patch [14]: [PATCH v4 14/17] KVM: arm64: Alloc pkvm_hyp_vcpu using pKVM heap allocator ---
commit b0e52464780692d209150dd491713c25a5d1c1c0
Author: Vincent Donnefort <[email protected]>

KVM: arm64: Alloc pkvm_hyp_vcpu using pKVM heap allocator
    
Transition the allocation of the hypervisor vCPU state structure (pkvm_hyp_vcpu)
from the host to the hypervisor using the new pKVM heap allocator. The host now
uses the pkvm_call_hyp_req wrapper to handle top-up requests if the hypervisor
runs out of heap memory.

> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> index a9b85ad37787a..3b29b3f43ffa5 100644
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c

[ ... ]

> @@ -153,28 +153,19 @@ static void __pkvm_destroy_hyp_vm(struct kvm *kvm)
>  
>  	kvm->arch.pkvm.handle = 0;
>  	kvm->arch.pkvm.is_created = false;
> -	free_hyp_memcache(&kvm->arch.pkvm.teardown_mc);
>  	free_hyp_memcache(&kvm->arch.pkvm.stage2_teardown_mc);
>  }
>  
>  static int __pkvm_create_hyp_vcpu(struct kvm_vcpu *vcpu)
>  {
> -	size_t hyp_vcpu_sz = PAGE_ALIGN(PKVM_HYP_VCPU_SIZE);
>  	pkvm_handle_t handle = vcpu->kvm->arch.pkvm.handle;
> -	void *hyp_vcpu;
>  	int ret;
>  
>  	init_hyp_stage2_memcache(&vcpu->arch.pkvm_memcache);
>  
> -	hyp_vcpu = alloc_pages_exact(hyp_vcpu_sz, GFP_KERNEL_ACCOUNT);
> -	if (!hyp_vcpu)
> -		return -ENOMEM;
> -
> -	ret = kvm_call_hyp_nvhe(__pkvm_init_vcpu, handle, vcpu, hyp_vcpu);
> +	ret = pkvm_call_hyp_req(__pkvm_init_vcpu, handle, vcpu);
[Severity: High]
Does this change unintentionally drop memory cgroup (kmemcg) accounting for
the vCPU state structure?

Previously, this structure was allocated with GFP_KERNEL_ACCOUNT. By switching
to pkvm_call_hyp_req() and relying on the EL2 heap top-up mechanism, it appears
the accounting is bypassed. When the top-up path calls pkvm_hyp_topup(), it uses
init_hyp_memcache() which lacks the HYP_MEMCACHE_ACCOUNT_KMEMCG flag. This
causes the host allocation in hyp_mc_alloc_fn() to fall back to GFP_KERNEL
instead of GFP_KERNEL_ACCOUNT.

Could this allow an unprivileged userspace process to bypass memcg limits by
repeatedly creating VMs and vCPUs, leading to a host out-of-memory condition?

While a previous commit noted that heap donations cannot be accounted for a
specific VM because they are shared and recycled, active VM and vCPU structures
remain in use for the lifetime of the VM and cannot be reclaimed by the new pKVM
shrinker. This might allow a malicious tenant to systematically exhaust host
memory.

>  	if (!ret)
>  		vcpu_set_flag(vcpu, VCPU_PKVM_FINALIZED);
> -	else
> -		free_pages_exact(hyp_vcpu, hyp_vcpu_sz);
>  
>  	return ret;
>  }

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