Re: [PATCH v3 09/17] KVM: arm64: Add selftests for the pKVM heap allocator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `__KVM_HOST_SMCCC_FUNC___pkvm_hyp_alloc_selftest` hypercall is incorrectly placed in the always-on band rather than the init-only band.
- [Medium] Physical pages are permanently leaked in the hypervisor if the allocator selftest encounters an error.
--

--- Patch [9]: [PATCH v3 09/17] KVM: arm64: Add selftests for the pKVM heap allocator ---
commit db8b74b081ab779ef5c70d80095da5b5809ca9d3
Author: Vincent Donnefort <vdonnefort@google.com>

KVM: arm64: Add selftests for the pKVM heap allocator

Introduce a comprehensive runtime selftest for the pKVM hypervisor heap
allocator, executed during init when CONFIG_NVHE_EL2_DEBUG is enabled.

> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 0c3126179704f..08466f608d1f5 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -92,6 +92,7 @@ enum __kvm_host_smccc_func {
>  	__KVM_HOST_SMCCC_FUNC___pkvm_hyp_topup,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_hyp_reclaim,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_hyp_reclaimable,
> +	__KVM_HOST_SMCCC_FUNC___pkvm_hyp_alloc_selftest,
>  
>  	MARKER(__KVM_HOST_SMCCC_FUNC_PKVM_ONLY),

[Severity: Medium]
Does this place the initialization-only test hypercall in the always-on band?

If __pkvm_hyp_alloc_selftest is only intended to run during initialization,
placing it here (before PKVM_ONLY) exposes it to the untrusted host
post-deprivilege. This allows the host to drive synchronous EL2 execution at
runtime. Should it be placed in the init-only band instead?

[ ... ]

> diff --git a/arch/arm64/kvm/hyp/nvhe/alloc.c b/arch/arm64/kvm/hyp/nvhe/alloc.c
> index 32c923d83f78b..49d7560ad150e 100644
> --- a/arch/arm64/kvm/hyp/nvhe/alloc.c
> +++ b/arch/arm64/kvm/hyp/nvhe/alloc.c

[ ... ]

> +	/* Test reclaiming the last chunk of the list */
> +	selftest_free(addr4);
> +	hyp_allocator_reclaim(allocator, &host_mc, SELFTEST_MAX_PAGES);
> +	if (host_mc.nr_pages != SELFTEST_MAX_PAGES - 3)
> +		return -EINVAL;
> +
> +	/* Test punching a hole in the middle of a free chunk ... */
> +	selftest_free(addr2);
> +	hyp_allocator_reclaim(allocator, &host_mc, SELFTEST_MAX_PAGES);
> +	if (host_mc.nr_pages != SELFTEST_MAX_PAGES - 2)
> +		return -EINVAL;
> +
> +	if (selftest_alloc(PAGE_SIZE))
> +		return -EINVAL;
> +	if (selftest_errno() != -ENOMEM)
> +		return -EINVAL;

[Severity: Medium]
Are these early return paths in hyp_allocator_selftest() leaking the physical
pages accumulated in the local host_mc variable?

If an error is encountered during the selftest and the function returns
early, host_mc goes out of scope. Because the physical pages populated
during the previous hyp_allocator_reclaim() calls are not returned to the
host or freed, they appear to be permanently leaked in EL2.

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