Re: [PATCH v6 03/18] KVM: arm64: Add pkvm_map_private_va_range

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: page table memory leak on error path in pkvm_map_private_va_range()
--

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

KVM: arm64: Add pkvm_map_private_va_range

In preparation for the pKVM heap allocator, introduce
pkvm_map_private_va_range() to map a physical range into the pKVM
private range.

> diff --git a/arch/arm64/kvm/hyp/nvhe/mm.c b/arch/arm64/kvm/hyp/nvhe/mm.c
> index 9ab7f613fa360..4a22fb74182be 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mm.c
[ ... ]
> @@ -90,6 +91,41 @@ int pkvm_alloc_private_va_range(size_t size, unsigned long *haddr)
>  	return ret;
>  }
>  
> +/**
> + * pkvm_map_private_va_range() - Map a physical range into the private VA range
> + * @haddr:	The virtual address in the private range.
> + * @phys:	The physical address to map.
> + * @size:	The size of the range to map.
> + *
> + * The hypervisor VA @haddr must have been first allocated with
> + * pkvm_alloc_private_va_range(). The created mapping can later be removed
> + * with pkvm_remove_mappings().
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int pkvm_map_private_va_range(void *haddr, phys_addr_t phys, size_t size)
> +{
> +	unsigned long addr = (unsigned long)haddr;
> +	int ret;
> +
> +	if (!PAGE_ALIGNED(addr | phys | size))
> +		return -EINVAL;
> +
> +	guard(hyp_spinlock)(&pkvm_pgd_lock);
> +
> +	if (addr < __io_map_base || addr >= __io_map_next)
> +		return -EINVAL;
> +
> +	if (size > __io_map_next - addr)
> +		return -EINVAL;
> +
> +	ret = kvm_pgtable_hyp_map(&pkvm_pgtable, addr, size, phys, PAGE_HYP);
> +	if (ret)
> +		kvm_pgtable_hyp_unmap(&pkvm_pgtable, addr, size);
[Severity: High]
Does this error path leak hypervisor page table memory?

If kvm_pgtable_hyp_map() fails midway (for instance, due to an out-of-memory
condition), pkvm_map_private_va_range() calls kvm_pgtable_hyp_unmap() to roll
back the partial mapping. The unmap walker will then encounter an invalid PTE
in the unmapped portion:

arch/arm64/kvm/hyp/pgtable.c:hyp_unmap_walker() {
    ...
    if (!kvm_pte_valid(ctx->old))
        return -EINVAL;
    ...
}

This causes the walker to abort. When the walk aborts, __kvm_pgtable_visit()
skips the KVM_PGTABLE_WALK_TABLE_POST callbacks for all parent tables:

arch/arm64/kvm/hyp/pgtable.c:__kvm_pgtable_visit() {
    ...
    if (!kvm_pgtable_walk_continue(data->walker, ret))
        goto out;

    if (ctx.flags & KVM_PGTABLE_WALK_TABLE_POST)
        ret = kvm_pgtable_visitor_cb(data, &ctx, ...);
    ...
}

Because the post-table callback is skipped, any intermediate page tables
allocated during the failed map are not freed. Since the hypervisor uses a
fixed memory pool, if an untrusted host can push the allocator to its limit
to trigger this error path, this leak could permanently deplete EL2 memory and
prevent further VM creation.

> +
> +	return ret;
> +}

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