Re: [PATCH] KVM: arm64: Validate the SVE vector length in pkvm_vcpu_init_sve()

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

Pre-existing issues:
- [Critical] A fallible memory allocation failure inside `hyp_pin_shared_mem()` triggers a `WARN_ON()`, which results in a fatal hypervisor panic.
--

--- Patch [1]: [PATCH] KVM: arm64: Validate the SVE vector length in pkvm_vcpu_init_sve() ---
commit f011c5300d433db59b3f2b97e822b9e148c39b84
Author: Fuad Tabba <[email protected]>
KVM: arm64: Validate the SVE vector length in pkvm_vcpu_init_sve()

This commit updates pkvm_vcpu_init_sve() to validate the host-provided
SVE vector length before deriving its state size. This prevents an invalid
length from reaching the sve_state_size_from_vl() macro and triggering a
fatal EL2 WARN_ON().

> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
[ ... ]
> @@ -460,14 +460,15 @@ static int pkvm_vcpu_init_sve(struct pkvm_hyp_vcpu *hyp_vcpu, struct kvm_vcpu *h
[ ... ]
> +	sve_state_size = sve_state_size_from_vl(sve_max_vl);
> +
>  	ret = hyp_pin_shared_mem(sve_state, sve_state + sve_state_size);

[Severity: Critical]
This is a pre-existing issue, but looking at hyp_pin_shared_mem(), could a
memory allocation failure here trigger a fatal hypervisor panic?

If host userspace repeatedly creates VMs and vCPUs to exhaust the
hypervisor's memory pool, pkvm_create_mappings_locked() can return
-ENOMEM. 

Because hyp_pin_shared_mem() wraps this fallible call in a WARN_ON():

arch/arm64/kvm/hyp/nvhe/mem_protect.c:hyp_pin_shared_mem() {
    ...
    for (cur = start; cur < end; cur += PAGE_SIZE) {
        p = hyp_virt_to_page(cur);
        hyp_page_ref_inc(p);
        if (p->refcount == 1)
            WARN_ON(pkvm_create_mappings_locked((void *)cur,
                                                (void *)cur + PAGE_SIZE,
                                                PAGE_HYP));
    }
    ...
}

And since WARN_ON() is fatal at EL2, does this allow unprivileged host
userspace to intentionally crash the host system if the allocation fails?

>  	if (ret)
>  		goto err;
>

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