Re: [PATCH v3 03/17] KVM: arm64: Add pkvm_map_private_va_range

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `pkvm_map_private_va_range` does not roll back partially created mappings if `kvm_pgtable_hyp_map` fails, causing Use-After-Free and isolation breaches.
- [Medium] Integer overflow in the bounds check of `pkvm_map_private_va_range` allows out-of-bounds requests to silently bypass validation.
--

--- Patch [3]: [PATCH v3 03/17] KVM: arm64: Add pkvm_map_private_va_range ---
commit c11c23d0f630b7a7c96e01ffbcb3dd1f881985c2
Author: Vincent Donnefort <vdonnefort@google.com>
Subject: KVM: arm64: Add pkvm_map_private_va_range

This commit introduces pkvm_map_private_va_range() to map a physical range
into the pKVM private VA range. This is in preparation for the upcoming pKVM
heap allocator.

> diff --git a/arch/arm64/kvm/hyp/nvhe/mm.c b/arch/arm64/kvm/hyp/nvhe/mm.c
> index 5cd2f1fe51e4c..20e4445dd57b7 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mm.c

[ ... ]

> @@ -90,6 +91,31 @@ 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()
> + *
> + * 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;
> +
> +	if (!PAGE_ALIGNED(addr | phys | size))
> +		return -EINVAL;
> +
> +	guard(hyp_spinlock)(&pkvm_pgd_lock);
> +
> +	if (addr < __hyp_private_va_start || addr + size > __io_map_base)
> +		return -EINVAL;

[Severity: Medium]
Could a very large size cause an integer overflow here? If size is large
enough, addr + size could wrap around to a small positive value. Since
__io_map_base is in the upper half of the address space, the wrapped sum
might be less than __io_map_base, bypassing this validation.

The subsequent kvm_pgtable_hyp_map() might then calculate a wrapped end
boundary, resulting in a zero-iteration loop that returns 0 (success)
without actually mapping anything. This could lead to a hypervisor panic
when the caller attempts to access the unmapped memory.

> +
> +	return kvm_pgtable_hyp_map(&pkvm_pgtable, addr, size, phys, PAGE_HYP);

[Severity: High]
Does this error path need to roll back partially created mappings?

If kvm_pgtable_hyp_map() fails midway, such as returning -ENOMEM when it
cannot allocate an intermediate page table, it appears to abort without
unmapping the leaf PTEs it has already installed.

Because this error is returned directly to the caller without a rollback,
the underlying physical pages might be freed back to the pool while
remaining mapped and writable in the EL2 page table.

Could this lead to use-after-free and isolation breaches when these physical
pages are reallocated?

> +}

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