[PATCH 0/3] KVM: arm64: Fix the hyp pool size on large machines, plus two hardening patches

[PATCH 0/3] KVM: arm64: Fix the hyp pool size on large machines, plus two hardening patches

[Xuheng Li]

Three changes for issues we found in pKVM as part of our research
project.

Patch 1 fixes a 32-bit size computation in hyp_pool_init(): a pool of
4 GiB or more gets a truncated range and maximum order.  hpool and
host_s2_pool reach that size on machines with about 2 TiB of RAM.

Patch 2 checks the host stage-2 PGD allocation for NULL before using
it.  With patch 1 applied that allocation cannot fail, so this only
turns a mis-sized pool into a clean -ENOMEM instead of a hyp panic.

Patch 3 makes hyp_map_walker() refuse to replace a valid block mapping
with a table, which would skip break-before-make and unmap the rest of
the block.  Nothing in mainline maps into an existing hyp block, so this
is hardening; Android's interface for EL2 modules to map pages can reach
it.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
Xuheng Li (3):
      KVM: arm64: Compute the hyp pool size in 64 bits
      KVM: arm64: Check the host stage-2 PGD allocation for NULL
      KVM: arm64: Don't replace a valid hyp block mapping with a table

 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++
 arch/arm64/kvm/hyp/nvhe/page_alloc.c  | 6 +++---
 arch/arm64/kvm/hyp/pgtable.c          | 7 +++++++
 3 files changed, 13 insertions(+), 3 deletions(-)
---
base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
change-id: 20261105-hyp-pool-and-map-b5e7be9a1191

Best regards,
-- 
Xuheng Li <xuheng@cs.columbia.edu>

[PATCH 0/3] KVM: arm64: Fix the hyp pool size on large machines, plus two hardening patches

[Xuheng Li via B4 Relay]

Three changes for issues we found in pKVM as part of our research
project.

Patch 1 fixes a 32-bit size computation in hyp_pool_init(): a pool of
4 GiB or more gets a truncated range and maximum order.  hpool and
host_s2_pool reach that size on machines with about 2 TiB of RAM.

Patch 2 checks the host stage-2 PGD allocation for NULL before using
it.  With patch 1 applied that allocation cannot fail, so this only
turns a mis-sized pool into a clean -ENOMEM instead of a hyp panic.

Patch 3 makes hyp_map_walker() refuse to replace a valid block mapping
with a table, which would skip break-before-make and unmap the rest of
the block.  Nothing in mainline maps into an existing hyp block, so this
is hardening; Android's interface for EL2 modules to map pages can reach
it.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
Xuheng Li (3):
      KVM: arm64: Compute the hyp pool size in 64 bits
      KVM: arm64: Check the host stage-2 PGD allocation for NULL
      KVM: arm64: Don't replace a valid hyp block mapping with a table

 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++
 arch/arm64/kvm/hyp/nvhe/page_alloc.c  | 6 +++---
 arch/arm64/kvm/hyp/pgtable.c          | 7 +++++++
 3 files changed, 13 insertions(+), 3 deletions(-)
---
base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
change-id: 20261105-hyp-pool-and-map-b5e7be9a1191

Best regards,
-- 
Xuheng Li <xuheng@cs.columbia.edu>

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

[Xuheng Li]

hyp_pool_init() takes the pool size as an unsigned int number of pages
and computes the size in bytes as nr_pages << PAGE_SHIFT, in 32 bits.
For a pool of 4 GiB or more (nr_pages >= 2^20 with 4 KiB pages) the
shift overflows, and both pool->range_end and pool->max_order are
derived from the truncated size.

The two large pools are sized from the amount of memory:
hyp_s1_pgtable_pages() for hpool and host_s2_pgtable_pages() for
host_s2_pool both provision page-granular tables for all of memory,
about one page per 2 MiB of RAM, so they reach 4 GiB on a machine with
about 2 TiB of RAM.  Arm systems of that size exist, for example AWS
Graviton4 x8g.metal-48xl instances (3 TiB) and Ampere Altra Max
servers (up to 4 TiB per socket).

With a truncated range_end, most of the pool lies outside
[range_start, range_end), so __hyp_attach_page() treats those pages as
external order-0 pages that never coalesce, and the pool cannot satisfy
multi-page allocations.  What that does depends on the host's IPA size,
since the only multi-page allocation from these pools is the host
stage-2 PGD in host_s2_zalloc_pages_exact():

 - With a 44- or 48-bit PARange the PGD is a single page.  pKVM boots,
   but both pools run with a wrong range and maximum order, and any
   later multi-page allocation from them would fail.

 - With a 42-bit PARange the PGD is eight concatenated level-1 tables.
   Its allocation fails, and host_s2_zalloc_pages_exact() calls
   hyp_split_page() on the result before checking it for NULL, so pKVM
   initialisation operates on a bogus struct hyp_page instead of failing
   with -ENOMEM.

Compute the size in 64 bits.

Fixes: 8e17c66249e9 ("KVM: arm64: Introduce a Hyp buddy page allocator")
Cc: stable@vger.kernel.org
Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/nvhe/page_alloc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/page_alloc.c b/arch/arm64/kvm/hyp/nvhe/page_alloc.c
index 57f86aa0f..e7c10d2f7 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;
 
 	/* Init the vmemmap portion */
 	p = hyp_phys_to_page(phys);

-- 
2.53.0

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

[Xuheng Li via B4 Relay]

From: Xuheng Li <xuheng@cs.columbia.edu>

hyp_pool_init() takes the pool size as an unsigned int number of pages
and computes the size in bytes as nr_pages << PAGE_SHIFT, in 32 bits.
For a pool of 4 GiB or more (nr_pages >= 2^20 with 4 KiB pages) the
shift overflows, and both pool->range_end and pool->max_order are
derived from the truncated size.

The two large pools are sized from the amount of memory:
hyp_s1_pgtable_pages() for hpool and host_s2_pgtable_pages() for
host_s2_pool both provision page-granular tables for all of memory,
about one page per 2 MiB of RAM, so they reach 4 GiB on a machine with
about 2 TiB of RAM.  Arm systems of that size exist, for example AWS
Graviton4 x8g.metal-48xl instances (3 TiB) and Ampere Altra Max
servers (up to 4 TiB per socket).

With a truncated range_end, most of the pool lies outside
[range_start, range_end), so __hyp_attach_page() treats those pages as
external order-0 pages that never coalesce, and the pool cannot satisfy
multi-page allocations.  What that does depends on the host's IPA size,
since the only multi-page allocation from these pools is the host
stage-2 PGD in host_s2_zalloc_pages_exact():

 - With a 44- or 48-bit PARange the PGD is a single page.  pKVM boots,
   but both pools run with a wrong range and maximum order, and any
   later multi-page allocation from them would fail.

 - With a 42-bit PARange the PGD is eight concatenated level-1 tables.
   Its allocation fails, and host_s2_zalloc_pages_exact() calls
   hyp_split_page() on the result before checking it for NULL, so pKVM
   initialisation operates on a bogus struct hyp_page instead of failing
   with -ENOMEM.

Compute the size in 64 bits.

Fixes: 8e17c66249e9 ("KVM: arm64: Introduce a Hyp buddy page allocator")
Cc: stable@vger.kernel.org
Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/nvhe/page_alloc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/page_alloc.c b/arch/arm64/kvm/hyp/nvhe/page_alloc.c
index 57f86aa0f..e7c10d2f7 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;
 
 	/* Init the vmemmap portion */
 	p = hyp_phys_to_page(phys);

-- 
2.53.0

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

[sashiko-bot]

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

[PATCH 2/3] KVM: arm64: Check the host stage-2 PGD allocation for NULL

[Xuheng Li]

host_s2_zalloc_pages_exact() passes the result of hyp_alloc_pages() to
hyp_split_page(hyp_virt_to_page(addr)) without checking it for NULL.
If the host stage-2 pool cannot supply the PGD, hyp_virt_to_page(NULL)
yields a bogus struct hyp_page, and hyp_split_page() reads and writes
through it, which ends in a hyp panic during pKVM initialisation
although kvm_pgtable_stage2_init() is prepared to fail with -ENOMEM.

The pool is sized so that this allocation, the first one made from it,
cannot fail, so nothing in mainline reaches this path once the pool
size is computed correctly.  Return NULL early anyway, so that a
mis-sized pool makes initialisation fail cleanly.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index d4c332a91..ee0848f2e 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -85,6 +85,9 @@ static void *host_s2_zalloc_pages_exact(size_t size)
 {
 	void *addr = hyp_alloc_pages(&host_s2_pool, get_order(size));
 
+	if (!addr)
+		return NULL;
+
 	hyp_split_page(hyp_virt_to_page(addr));
 
 	/*

-- 
2.53.0

[PATCH 2/3] KVM: arm64: Check the host stage-2 PGD allocation for NULL

[Xuheng Li via B4 Relay]

From: Xuheng Li <xuheng@cs.columbia.edu>

host_s2_zalloc_pages_exact() passes the result of hyp_alloc_pages() to
hyp_split_page(hyp_virt_to_page(addr)) without checking it for NULL.
If the host stage-2 pool cannot supply the PGD, hyp_virt_to_page(NULL)
yields a bogus struct hyp_page, and hyp_split_page() reads and writes
through it, which ends in a hyp panic during pKVM initialisation
although kvm_pgtable_stage2_init() is prepared to fail with -ENOMEM.

The pool is sized so that this allocation, the first one made from it,
cannot fail, so nothing in mainline reaches this path once the pool
size is computed correctly.  Return NULL early anyway, so that a
mis-sized pool makes initialisation fail cleanly.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index d4c332a91..ee0848f2e 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -85,6 +85,9 @@ static void *host_s2_zalloc_pages_exact(size_t size)
 {
 	void *addr = hyp_alloc_pages(&host_s2_pool, get_order(size));
 
+	if (!addr)
+		return NULL;
+
 	hyp_split_page(hyp_virt_to_page(addr));
 
 	/*

-- 
2.53.0

[PATCH 3/3] KVM: arm64: Don't replace a valid hyp block mapping with a table

[Xuheng Li]

When kvm_pgtable_hyp_map() is asked to map part of a range that a valid
block mapping already covers, hyp_map_walker_try_leaf() declines, as the
request cannot be a leaf at the block's level.  hyp_map_walker() then
allocates a zeroed table and stores it over the valid block entry.  The
walk descends into the new table and maps the requested pages, but the
rest of the block is now unmapped, and the valid block was replaced
without break-before-make.

No in-tree caller maps into an existing hyp block mapping today, so this
cannot be triggered in mainline.  Fail such a request with -EEXIST
instead, so that a future caller gets an error rather than silently
losing the rest of the block.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/pgtable.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 9c83837b1..4c731b101 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -434,6 +434,13 @@ static int hyp_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
 	if (WARN_ON(ctx->level == KVM_PGTABLE_LAST_LEVEL))
 		return -EINVAL;
 
+	/*
+	 * Replacing a valid block with a table would need break-before-make,
+	 * and the new table would leave the rest of the block unmapped.
+	 */
+	if (WARN_ON(kvm_pte_valid(ctx->old)))
+		return -EEXIST;
+
 	childp = (kvm_pte_t *)mm_ops->zalloc_page(NULL);
 	if (!childp)
 		return -ENOMEM;

-- 
2.53.0

[PATCH 3/3] KVM: arm64: Don't replace a valid hyp block mapping with a table

[Xuheng Li via B4 Relay]

From: Xuheng Li <xuheng@cs.columbia.edu>

When kvm_pgtable_hyp_map() is asked to map part of a range that a valid
block mapping already covers, hyp_map_walker_try_leaf() declines, as the
request cannot be a leaf at the block's level.  hyp_map_walker() then
allocates a zeroed table and stores it over the valid block entry.  The
walk descends into the new table and maps the requested pages, but the
rest of the block is now unmapped, and the valid block was replaced
without break-before-make.

No in-tree caller maps into an existing hyp block mapping today, so this
cannot be triggered in mainline.  Fail such a request with -EEXIST
instead, so that a future caller gets an error rather than silently
losing the rest of the block.

Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/pgtable.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 9c83837b1..4c731b101 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -434,6 +434,13 @@ static int hyp_map_walker(const struct kvm_pgtable_visit_ctx *ctx,
 	if (WARN_ON(ctx->level == KVM_PGTABLE_LAST_LEVEL))
 		return -EINVAL;
 
+	/*
+	 * Replacing a valid block with a table would need break-before-make,
+	 * and the new table would leave the rest of the block unmapped.
+	 */
+	if (WARN_ON(kvm_pte_valid(ctx->old)))
+		return -EEXIST;
+
 	childp = (kvm_pte_t *)mm_ops->zalloc_page(NULL);
 	if (!childp)
 		return -ENOMEM;

-- 
2.53.0