[PATCH] KVM: arm64: Only free fully unmapped tables in stage2_free_walker()

[PATCH] KVM: arm64: Only free fully unmapped tables in stage2_free_walker()

[Oliver Upton]

syzkaller caught a terminal use-after-free in the free walker resulting
from a premature put_page() on a partially unmapped page table.
Previously KVM performed a single walk covering the entire IPA space,
but now that the range of the walk is up to KVM_PGTABLE_MIN_BLOCK_LEVEL
worth of memory it is possibly to only partially free a table.

Fix it by only dropping the table reference if the page count of the
table is 1 (i.e. no longer contains valid PTEs).

Fixes: e9abe311f356 ("KVM: arm64: Reschedule as needed when destroying the stage-2 page-tables")
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/hyp/pgtable.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index c36f282a175d..50b8fb7cc59f 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -1535,20 +1535,41 @@ size_t kvm_pgtable_stage2_pgd_size(u64 vtcr)
 	return kvm_pgd_pages(ia_bits, start_level) * PAGE_SIZE;
 }
 
-static int stage2_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
-			      enum kvm_pgtable_walk_flags visit)
+static int stage2_free_leaf(const struct kvm_pgtable_visit_ctx *ctx)
 {
 	struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
 
-	if (!stage2_pte_is_counted(ctx->old))
+	mm_ops->put_page(ctx->ptep);
+	return 0;
+}
+
+static int stage2_free_table_post(const struct kvm_pgtable_visit_ctx *ctx)
+{
+	struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
+	kvm_pte_t *childp = kvm_pte_follow(ctx->old, mm_ops);
+
+	if (mm_ops->page_count(childp) != 1)
 		return 0;
 
 	mm_ops->put_page(ctx->ptep);
+	mm_ops->put_page(childp);
+	return 0;
+}
 
-	if (kvm_pte_table(ctx->old, ctx->level))
-		mm_ops->put_page(kvm_pte_follow(ctx->old, mm_ops));
+static int stage2_free_walker(const struct kvm_pgtable_visit_ctx *ctx,
+			      enum kvm_pgtable_walk_flags visit)
+{
+	if (!stage2_pte_is_counted(ctx->old))
+		return 0;
 
-	return 0;
+	switch (visit) {
+	case KVM_PGTABLE_WALK_LEAF:
+		return stage2_free_leaf(ctx);
+	case KVM_PGTABLE_WALK_TABLE_POST:
+		return stage2_free_table_post(ctx);
+	default:
+		return -EINVAL;
+	}
 }
 
 void kvm_pgtable_stage2_destroy_range(struct kvm_pgtable *pgt,
-- 
2.39.5

Re: [PATCH] KVM: arm64: Only free fully unmapped tables in stage2_free_walker()

[Marc Zyngier]

On Thu, 04 Sep 2025 11:17:46 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> syzkaller caught a terminal use-after-free in the free walker resulting
> from a premature put_page() on a partially unmapped page table.
> Previously KVM performed a single walk covering the entire IPA space,
> but now that the range of the walk is up to KVM_PGTABLE_MIN_BLOCK_LEVEL
> worth of memory it is possibly to only partially free a table.
> 
> Fix it by only dropping the table reference if the page count of the
> table is 1 (i.e. no longer contains valid PTEs).
> 
> Fixes: e9abe311f356 ("KVM: arm64: Reschedule as needed when destroying the stage-2 page-tables")
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH] KVM: arm64: Only free fully unmapped tables in stage2_free_walker()

[Alexander Potapenko]

On Fri, Sep 5, 2025 at 9:32 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Thu, 04 Sep 2025 11:17:46 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > syzkaller caught a terminal use-after-free in the free walker resulting
> > from a premature put_page() on a partially unmapped page table.
> > Previously KVM performed a single walk covering the entire IPA space,
> > but now that the range of the walk is up to KVM_PGTABLE_MIN_BLOCK_LEVEL
> > worth of memory it is possibly to only partially free a table.
> >
> > Fix it by only dropping the table reference if the page count of the
> > table is 1 (i.e. no longer contains valid PTEs).
> >
> > Fixes: e9abe311f356 ("KVM: arm64: Reschedule as needed when destroying the stage-2 page-tables")
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
Tested-by: Alexander Potapenko <glider@google.com>

Re: [PATCH] KVM: arm64: Only free fully unmapped tables in stage2_free_walker()

[Oliver Upton]

On Thu, 04 Sep 2025 03:17:46 -0700, Oliver Upton wrote:
> syzkaller caught a terminal use-after-free in the free walker resulting
> from a premature put_page() on a partially unmapped page table.
> Previously KVM performed a single walk covering the entire IPA space,
> but now that the range of the walk is up to KVM_PGTABLE_MIN_BLOCK_LEVEL
> worth of memory it is possibly to only partially free a table.
> 
> Fix it by only dropping the table reference if the page count of the
> table is 1 (i.e. no longer contains valid PTEs).
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: Only free fully unmapped tables in stage2_free_walker()
      https://git.kernel.org/kvmarm/kvmarm/c/3095864771d8

--
Best,
Oliver