Re: [PATCH 1/3] KVM: arm64: Compute the hyp pool size in 64 bits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: arm64: 32-bit shift overflow in hyp_early_alloc_contig() for 4 GiB+ pools
--

commit 72d17cd9f535bf2996e0d4ac086eb07a7f6f29ba
Author: Xuheng Li <xuheng@cs.columbia.edu>

KVM: arm64: Compute the hyp pool size in 64 bits

This commit fixes a 32-bit shift overflow in hyp_pool_init() when computing
the byte size of a hypervisor page pool from nr_pages << PAGE_SHIFT on
systems where pool size reaches 4 GiB or more.

> diff --git a/arch/arm64/kvm/hyp/nvhe/page_alloc.c b/arch/arm64/kvm/hyp/nvhe/page_alloc.c
> index 57f86aa0f82fc..e7c10d2f7b0bf 100644
> --- a/arch/arm64/kvm/hyp/nvhe/page_alloc.c
> +++ b/arch/arm64/kvm/hyp/nvhe/page_alloc.c
> @@ -233,16 +233,16 @@ int hyp_pool_init(struct hyp_pool *pool, u64 pfn, unsigned int nr_pages,
>  		  unsigned int reserved_pages)
>  {
>  	phys_addr_t phys = hyp_pfn_to_phys(pfn);
> +	u64 size = (u64)nr_pages << PAGE_SHIFT;
>  	struct hyp_page *p;
>  	int i;
>  
>  	hyp_spin_lock_init(&pool->lock);
> -	pool->max_order = min(MAX_PAGE_ORDER,
> -			      get_order(nr_pages << PAGE_SHIFT));
> +	pool->max_order = min(MAX_PAGE_ORDER, get_order(size));
>  	for (i = 0; i <= pool->max_order; i++)
>  		INIT_LIST_HEAD(&pool->free_area[i]);
>  	pool->range_start = phys;
> -	pool->range_end = phys + (nr_pages << PAGE_SHIFT);
> +	pool->range_end = phys + size;

[Severity: High]
Does hyp_early_alloc_contig() need the same 64-bit cast when carving out
the backing memory for these pools?

Before hyp_pool_init() is called in __pkvm_init_finalise() and
prepare_s2_pool(), __pkvm_init() -> divide_memory_pool() allocates
vmemmap_base, hyp_pgt_base, and host_s2_pgt_base via
hyp_early_alloc_contig():

arch/arm64/kvm/hyp/nvhe/setup.c:divide_memory_pool() {
	...
	nr_pages = hyp_vmemmap_pages(sizeof(struct hyp_page));
	vmemmap_base = hyp_early_alloc_contig(nr_pages);
	if (!vmemmap_base)
		return -ENOMEM;
	...
	nr_pages = hyp_s1_pgtable_pages();
	hyp_pgt_base = hyp_early_alloc_contig(nr_pages);
	if (!hyp_pgt_base)
		return -ENOMEM;

	nr_pages = host_s2_pgtable_pages();
	host_s2_pgt_base = hyp_early_alloc_contig(nr_pages);
	...
}

In hyp_early_alloc_contig(), nr_pages is also an unsigned int shifted in
32 bits before conversion to unsigned long:

arch/arm64/kvm/hyp/nvhe/early_alloc.c:hyp_early_alloc_contig() {
	unsigned long size = (nr_pages << PAGE_SHIFT);
	void *ret = (void *)cur;

	if (!nr_pages)
		return NULL;

	if (end - cur < size)
		return NULL;

	cur += size;
	memset(ret, 0, size);

	return ret;
}

When nr_pages reaches 4 GiB worth of pages on systems with around 2 TiB or
more of RAM, size in hyp_early_alloc_contig() overflows in 32 bits and cur
advances by the truncated size, placing vm_table_base, hyp_pgt_base,
host_s2_pgt_base, and ffa_proxy_pages at overlapping physical addresses.
With hyp_pool_init() now initializing hpool and host_s2_pool across the
full 64-bit size, won't the pools overrun their truncated early allocations
and corrupt the adjacent regions during pKVM initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-hyp-pool-and-map-v1-0-756bb5d8e067@cs.columbia.edu?part=1