[PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Karl Mehltretter]

A vCPU in L2 caches its shadow S2 MMU in vcpu->arch.hw_mmu.
kvm_vcpu_init_nested() can grow kvm->arch.nested_mmus while initialising
another vCPU: it copies the MMUs, publishes the new allocation, and frees
the old one.  It updates pgt->mmu back-pointers, but not hw_mmu, leaving
the first vCPU with a pointer to freed memory.  hw_mmu cannot be fixed up
the same way: a running vCPU reads it without holding mmu_lock.  Copying
also duplicates the MMU's refcount, leaving the live copy permanently
elevated.

KASAN reports this as a slab-use-after-free in kvm_handle_guest_abort().

Make nested_mmus a pointer table and allocate each MMU separately.
Growing the table now moves only pointer entries, preserving cached
hw_mmu pointers, pgt->mmu back-pointers, and each MMU's refcount.  Fully
initialise new MMUs before publishing the table and its size under
mmu_lock.

The old failure path passed uninitialised entries to
kvm_free_stage2_pgd(), which needs mmu->arch.  Use an allocation helper
that returns only fully initialised MMUs, so error cleanup frees only
completed objects; kvm_init_stage2_mmu() unwinds a failed initialisation.

Fixes: 4f128f8e1aaa ("KVM: arm64: nv: Support multiple nested Stage-2 mmu structures")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Tested on an arm64 KASAN kernel under QEMU TCG with EL2 emulation
(-machine virt,virtualization=on -cpu max, kvm-arm.mode=nested): the
selftest in patch 2 reports the slab-use-after-free without this patch
and passes with it.

 arch/arm64/include/asm/kvm_host.h |  6 +-
 arch/arm64/kvm/nested.c           | 92 +++++++++++++++++++++----------
 2 files changed, 67 insertions(+), 31 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bae2c4f92ef5..f587b01039f9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -319,10 +319,10 @@ struct kvm_arch {
 	u64 fgu[__NR_FGT_GROUP_IDS__];
 
 	/*
-	 * Stage 2 paging state for VMs with nested S2 using a virtual
-	 * VMID.
+	 * Stage 2 paging state for VMs with nested S2 using a virtual VMID.
+	 * MMUs are individually allocated to keep their addresses stable.
 	 */
-	struct kvm_s2_mmu *nested_mmus;
+	struct kvm_s2_mmu **nested_mmus;
 	size_t nested_mmus_size;
 	int nested_mmus_next;
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dfb96edbdc43..af804a5ddca7 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/err.h>
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
 
@@ -66,11 +67,36 @@ static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
 	return kvm_init_stage2_mmu(kvm, mmu, kvm_get_pa_bits(kvm));
 }
 
+static struct kvm_s2_mmu *alloc_nested_s2_mmu(struct kvm *kvm)
+{
+	struct kvm_s2_mmu *mmu;
+	int ret;
+
+	mmu = kzalloc_obj(*mmu, GFP_KERNEL_ACCOUNT);
+	if (!mmu)
+		return ERR_PTR(-ENOMEM);
+
+	ret = init_nested_s2_mmu(kvm, mmu);
+	if (ret) {
+		/* kvm_init_stage2_mmu() frees its internal allocations on error */
+		kfree(mmu);
+		return ERR_PTR(ret);
+	}
+
+	return mmu;
+}
+
+static void free_nested_s2_mmu(struct kvm_s2_mmu *mmu)
+{
+	kvm_free_stage2_pgd(mmu);
+	kfree(mmu);
+}
+
 int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = vcpu->kvm;
-	struct kvm_s2_mmu *tmp;
-	int num_mmus, ret = 0;
+	struct kvm_s2_mmu **tmp;
+	int i, num_mmus, ret = 0;
 
 	if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features) &&
 	    !cpus_have_final_cap(ARM64_HAS_HCR_NV1))
@@ -96,38 +122,48 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
 		if (!tmp)
 			return -ENOMEM;
 
+		/*
+		 * Populate new slots before publishing: table walkers hold
+		 * mmu_lock and iterate up to nested_mmus_size.
+		 */
+		for (i = kvm->arch.nested_mmus_size; i < num_mmus; i++) {
+			struct kvm_s2_mmu *mmu = alloc_nested_s2_mmu(kvm);
+
+			if (IS_ERR(mmu)) {
+				ret = PTR_ERR(mmu);
+				break;
+			}
+
+			tmp[i] = mmu;
+		}
+
+		if (ret) {
+			while (i-- > kvm->arch.nested_mmus_size)
+				free_nested_s2_mmu(tmp[i]);
+
+			kvfree(tmp);
+
+			free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
+			vcpu->arch.ctxt.vncr_array = NULL;
+
+			return ret;
+		}
+
 		write_lock(&kvm->mmu_lock);
 
 		if (kvm->arch.nested_mmus_size) {
 			memcpy(tmp, kvm->arch.nested_mmus,
 			       size_mul(sizeof(*tmp), kvm->arch.nested_mmus_size));
-
-			for (int i = 0; i < kvm->arch.nested_mmus_size; i++)
-				tmp[i].pgt->mmu = &tmp[i];
 		}
 
 		swap(kvm->arch.nested_mmus, tmp);
+		kvm->arch.nested_mmus_size = num_mmus;
 
 		write_unlock(&kvm->mmu_lock);
 
 		kvfree(tmp);
 	}
 
-	for (int i = kvm->arch.nested_mmus_size; !ret && i < num_mmus; i++)
-		ret = init_nested_s2_mmu(kvm, &kvm->arch.nested_mmus[i]);
-
-	if (ret) {
-		for (int i = kvm->arch.nested_mmus_size; i < num_mmus; i++)
-			kvm_free_stage2_pgd(&kvm->arch.nested_mmus[i]);
-
-		free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
-		vcpu->arch.ctxt.vncr_array = NULL;
-
-		return ret;
-	}
-
-	kvm->arch.nested_mmus_size = num_mmus;
-
 	return 0;
 }
 
@@ -725,7 +761,7 @@ void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
 	write_lock(&kvm->mmu_lock);
 
 	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (!kvm_s2_mmu_valid(mmu))
 			continue;
@@ -767,7 +803,7 @@ struct kvm_s2_mmu *lookup_s2_mmu(struct kvm_vcpu *vcpu)
 	 *   if S2 translation is disabled.
 	 */
 	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (!kvm_s2_mmu_valid(mmu))
 			continue;
@@ -806,7 +842,7 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
 	for (i = kvm->arch.nested_mmus_next;
 	     i < (kvm->arch.nested_mmus_size + kvm->arch.nested_mmus_next);
 	     i++) {
-		s2_mmu = &kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
+		s2_mmu = kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
 
 		if (atomic_read(&s2_mmu->refcnt) == 0)
 			break;
@@ -1223,7 +1259,7 @@ void kvm_nested_s2_wp(struct kvm *kvm)
 		return;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
 			kvm_stage2_wp_range(mmu, 0, kvm_phys_size(mmu));
@@ -1242,7 +1278,7 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
 		return;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
 			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
@@ -1261,7 +1297,7 @@ void kvm_nested_s2_flush(struct kvm *kvm)
 		return;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
 			kvm_stage2_flush_range(mmu, 0, kvm_phys_size(mmu));
@@ -1273,10 +1309,10 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
 	int i;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (!WARN_ON(atomic_read(&mmu->refcnt)))
-			kvm_free_stage2_pgd(mmu);
+			free_nested_s2_mmu(mmu);
 	}
 	kvfree(kvm->arch.nested_mmus);
 	kvm->arch.nested_mmus = NULL;

base-commit: 38436106b2f5ceb55950a7098c1a5804de2bde62
-- 
2.39.5 (Apple Git-154)

[PATCH 2/2] KVM: arm64: selftests: Add a nested S2 MMU realloc test

[Karl Mehltretter]

Add a regression test for a stale vcpu->arch.hw_mmu reference when
initialising a vCPU grows the nested S2 MMU table.

The test drives vCPU0 into L2 through a minimal L1 stage-2 identity map,
pins it to a second pCPU where it spins in L2, and then initialises
vCPU1.  That initialisation grows the nested MMU table while vCPU0 still
holds one of its entries; keeping vCPU0 on a pCPU of its own means the
reference stays live without relying on hw_mmu being retained across a
schedule-out.  vCPU0 is then released and has to run to completion.

Creating vCPU1 up front is what allows the in-kernel VGIC to be used:
kvm_arch_vcpu_precreate() refuses KVM_CREATE_VCPU once the VGIC has been
initialised, which the test does before its first KVM_RUN.  Creation on
its own increments online_vcpus, so deferring vCPU1's KVM_ARM_VCPU_INIT
until vCPU0 is in L2 still grows the table.

With KASAN enabled, an unfixed kernel reports a slab-use-after-free in
kvm_handle_guest_abort(); with the fix it completes cleanly.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

The test requires nested virtualization and two pCPUs.  Under QEMU TCG
it takes ~233s, exceeding the 120s timeout in
tools/testing/selftests/kvm/settings; psci_test takes ~167s in the same
boot, so emulating the second vCPU is the dominant cost.

 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../kvm/arm64/nested_mmu_realloc_test.c       | 278 ++++++++++++++++++
 2 files changed, 279 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/arm64/nested_mmu_realloc_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 6fc34e9bf8e1..222f37cbeec8 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -175,6 +175,7 @@ TEST_GEN_PROGS_arm64 += arm64/host_sve
 TEST_GEN_PROGS_arm64 += arm64/hypercalls
 TEST_GEN_PROGS_arm64 += arm64/external_aborts
 TEST_GEN_PROGS_arm64 += arm64/mmio_sign_ext
+TEST_GEN_PROGS_arm64 += arm64/nested_mmu_realloc_test
 TEST_GEN_PROGS_arm64 += arm64/page_fault_test
 TEST_GEN_PROGS_arm64 += arm64/psci_test
 TEST_GEN_PROGS_arm64 += arm64/sea_to_user
diff --git a/tools/testing/selftests/kvm/arm64/nested_mmu_realloc_test.c b/tools/testing/selftests/kvm/arm64/nested_mmu_realloc_test.c
new file mode 100644
index 000000000000..5eaf24106eee
--- /dev/null
+++ b/tools/testing/selftests/kvm/arm64/nested_mmu_realloc_test.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Regression test for a stale vcpu->arch.hw_mmu pointer when the nested
+ * stage-2 MMU table grows while another vCPU is running in L2.  On affected
+ * kernels, KASAN detects the use-after-free on the first post-resize L2 exit.
+ */
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+#include "ucall.h"
+
+#include <asm/ptrace.h>
+#include <asm/sysreg.h>
+#include <errno.h>
+#include <pthread.h>
+#include <sched.h>
+
+enum {
+	STAGE_L1_STARTED,
+	STAGE_L1_S2_READY,
+	STAGE_L2_ENTERED,
+};
+
+/* A 36-bit IPA and 1GB blocks give L1 a 64-entry stage-2 root table. */
+#define L1_S2_IPA_BITS		36
+#define L1_S2_BLOCK_SHIFT	30
+#define L1_S2_BLOCK_SIZE	BIT_ULL(L1_S2_BLOCK_SHIFT)
+#define L1_S2_ROOT_ENTRIES	BIT(L1_S2_IPA_BITS - L1_S2_BLOCK_SHIFT)
+#define L1_S2_SL0		1
+#define L1_S2_VTCR		(VTCR_EL2_RES1 |					\
+				 FIELD_PREP(VTCR_EL2_T0SZ, 64 - L1_S2_IPA_BITS) |	\
+				 FIELD_PREP(VTCR_EL2_SL0, L1_S2_SL0) |		\
+				 FIELD_PREP(VTCR_EL2_IRGN0, VTCR_EL2_IRGN0_WBWA) |	\
+				 FIELD_PREP(VTCR_EL2_ORGN0, VTCR_EL2_ORGN0_WBWA) |	\
+				 FIELD_PREP(VTCR_EL2_SH0, VTCR_EL2_SH0_INNER) |	\
+				 FIELD_PREP(VTCR_EL2_TG0, VTCR_EL2_TG0_4K) |		\
+				 FIELD_PREP(VTCR_EL2_PS, ID_AA64MMFR0_EL1_PARANGE_36))
+#define L1_S2_MEMATTR_NORMAL	(0xfULL << 2)
+#define L1_S2_S2AP_R		BIT(6)
+#define L1_S2_S2AP_W		BIT(7)
+#define L1_S2_BLOCK_DESC(pa)	(((pa) & GENMASK_ULL(47, L1_S2_BLOCK_SHIFT)) |	\
+				 L1_S2_MEMATTR_NORMAL | L1_S2_S2AP_R |		\
+				 L1_S2_S2AP_W | PTE_SHARED | PTE_AF | PTE_VALID)
+
+struct test_state {
+	u32 running;
+	u32 release;
+	u32 resumed;
+	u32 done;
+};
+
+struct vcpu_thread_args {
+	struct kvm_vcpu *vcpu;
+	int cpu;
+};
+
+#define copy_el2_to_el1(reg)						\
+	write_sysreg_s(read_sysreg_s(SYS_##reg##_EL1), SYS_##reg##_EL12)
+
+static void l2_guest(struct test_state *state)
+{
+	GUEST_ASSERT_EQ(get_current_el(), 1);
+	GUEST_SYNC(STAGE_L2_ENTERED);
+
+	WRITE_ONCE(state->running, 1);
+	while (!READ_ONCE(state->release))
+		cpu_relax();
+
+	WRITE_ONCE(state->resumed, 1);
+	while (!READ_ONCE(state->done))
+		cpu_relax();
+
+	GUEST_DONE();
+}
+
+static void l1_guest(u64 l2_pc, u64 state_gva, u64 s2_root_gpa)
+{
+	u64 sp;
+
+	GUEST_SYNC(STAGE_L1_STARTED);
+
+	copy_el2_to_el1(SCTLR);
+	copy_el2_to_el1(MAIR);
+	copy_el2_to_el1(TCR);
+	copy_el2_to_el1(TTBR0);
+	copy_el2_to_el1(TTBR1);
+
+	asm volatile("mov %0, sp" : "=r" (sp));
+	write_sysreg(sp, sp_el1);
+
+	write_sysreg(l2_pc, elr_el2);
+	write_sysreg(PSR_MODE_EL1h | PSR_D_BIT | PSR_A_BIT |
+		     PSR_I_BIT | PSR_F_BIT, spsr_el2);
+
+	write_sysreg(s2_root_gpa, vttbr_el2);
+	write_sysreg(L1_S2_VTCR, vtcr_el2);
+	isb();
+
+	GUEST_SYNC(STAGE_L1_S2_READY);
+
+	sysreg_clear_set(hcr_el2, HCR_EL2_TGE, HCR_EL2_VM);
+	isb();
+
+	asm volatile("mov x0, %0\n"
+		     "eret\n"
+		     :
+		     : "r" (state_gva)
+		     : "x0", "memory");
+
+	GUEST_ASSERT(0);
+}
+
+/* Sleep to avoid competing with the vCPU; the runner times out stalled tests. */
+static void wait_for_u32(u32 *ptr, u32 val)
+{
+	while (READ_ONCE(*ptr) != val)
+		usleep(1000);
+}
+
+static int pick_two_cpus(int *first_cpu, int *second_cpu)
+{
+	cpu_set_t allowed_mask;
+	int ret;
+
+	ret = sched_getaffinity(0, sizeof(allowed_mask), &allowed_mask);
+	TEST_ASSERT(!ret, "sched_getaffinity() failed, errno=%d", errno);
+
+	*first_cpu = -1;
+	*second_cpu = -1;
+
+	for (int cpu = 0; cpu < CPU_SETSIZE; cpu++) {
+		if (!CPU_ISSET(cpu, &allowed_mask))
+			continue;
+
+		if (*first_cpu < 0)
+			*first_cpu = cpu;
+		else
+			*second_cpu = cpu;
+
+		if (*second_cpu >= 0)
+			return 1;
+	}
+
+	return 0;
+}
+
+/* Mirror KVM's fallback from TGRAN4_2 to TGRAN4. */
+static bool l1_s2_supports_4k(struct kvm_vcpu *vcpu)
+{
+	u64 mmfr0 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR0_EL1));
+	u64 s2 = SYS_FIELD_GET(ID_AA64MMFR0_EL1, TGRAN4_2, mmfr0);
+
+	if (s2 == ID_AA64MMFR0_EL1_TGRAN4_2_TGRAN4)
+		return SYS_FIELD_GET(ID_AA64MMFR0_EL1, TGRAN4, mmfr0) !=
+		       ID_AA64MMFR0_EL1_TGRAN4_NI;
+
+	return s2 != ID_AA64MMFR0_EL1_TGRAN4_2_NI;
+}
+
+static void build_l1_s2_idmap(struct kvm_vm *vm, gpa_t root_gpa)
+{
+	u64 *root = addr_gpa2hva(vm, root_gpa);
+
+	for (int i = 0; i < L1_S2_ROOT_ENTRIES; i++)
+		root[i] = L1_S2_BLOCK_DESC((u64)i * L1_S2_BLOCK_SIZE);
+}
+
+static void run_to_sync_stage(struct kvm_vcpu *vcpu, u64 expected_stage)
+{
+	struct ucall uc;
+
+	vcpu_run(vcpu);
+
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_SYNC:
+		TEST_ASSERT_EQ(uc.args[1], expected_stage);
+		return;
+	case UCALL_ABORT:
+		REPORT_GUEST_ASSERT(uc);
+		return;
+	default:
+		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
+	}
+}
+
+static void *vcpu_thread_main(void *data)
+{
+	struct vcpu_thread_args *args = data;
+	struct kvm_vcpu *vcpu = args->vcpu;
+	struct ucall uc;
+
+	pin_self_to_cpu(args->cpu);
+
+	vcpu_run(vcpu);
+
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_DONE:
+		return NULL;
+	case UCALL_ABORT:
+		REPORT_GUEST_ASSERT(uc);
+		return NULL;
+	default:
+		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
+	}
+}
+
+int main(void)
+{
+	struct vcpu_thread_args thread_args;
+	struct kvm_vcpu_init init;
+	struct test_state *state;
+	struct kvm_vcpu *vcpu0;
+	struct kvm_vcpu *vcpu1;
+	struct kvm_vm *vm;
+	pthread_t thread;
+	gva_t state_gva;
+	gpa_t s2_root_gpa;
+	int ctrl_cpu, vcpu_cpu, ret;
+
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_ARM_EL2));
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE) >= L1_S2_IPA_BITS);
+
+	TEST_REQUIRE(pick_two_cpus(&ctrl_cpu, &vcpu_cpu));
+	pin_self_to_cpu(ctrl_cpu);
+	pr_info("Running control thread on pCPU %d, vCPU thread on pCPU %d\n",
+		ctrl_cpu, vcpu_cpu);
+
+	vm = vm_create(2);
+
+	kvm_get_default_vcpu_target(vm, &init);
+	init.features[0] |= BIT(KVM_ARM_VCPU_HAS_EL2);
+
+	vcpu0 = aarch64_vcpu_add(vm, 0, &init, l1_guest);
+	TEST_REQUIRE(l1_s2_supports_4k(vcpu0));
+
+	vcpu1 = __vm_vcpu_add(vm, 1);
+	state_gva = vm_alloc_page(vm);
+	state = addr_gva2hva(vm, state_gva);
+	*state = (struct test_state) {};
+	s2_root_gpa = vm_phy_page_alloc(vm, 0, vm->memslots[MEM_REGION_TEST_DATA]);
+	build_l1_s2_idmap(vm, s2_root_gpa);
+
+	vcpu_args_set(vcpu0, 3, (u64)l2_guest, state_gva, s2_root_gpa);
+	kvm_arch_vm_finalize_vcpus(vm);
+
+	run_to_sync_stage(vcpu0, STAGE_L1_STARTED);
+	run_to_sync_stage(vcpu0, STAGE_L1_S2_READY);
+	run_to_sync_stage(vcpu0, STAGE_L2_ENTERED);
+
+	thread_args = (struct vcpu_thread_args) {
+		.vcpu = vcpu0,
+		.cpu = vcpu_cpu,
+	};
+	ret = pthread_create(&thread, NULL, vcpu_thread_main, &thread_args);
+	TEST_ASSERT(!ret, "Failed to create vCPU thread, ret=%d", ret);
+
+	wait_for_u32(&state->running, 1);
+	pr_info("vCPU0 is running in L2; initializing vCPU1 to grow the table\n");
+
+	/*
+	 * vCPU0 holds a nested MMU while vCPU1 initialization grows the table.
+	 * vCPU1 was created before VGIC initialization because KVM_CREATE_VCPU is
+	 * refused afterwards.
+	 */
+	aarch64_vcpu_setup(vcpu1, &init);
+
+	pr_info("vCPU1 initialized; releasing vCPU0\n");
+	WRITE_ONCE(state->release, 1);
+	wait_for_u32(&state->resumed, 1);
+	pr_info("vCPU0 resumed after nested MMU resize\n");
+
+	WRITE_ONCE(state->done, 1);
+	ret = pthread_join(thread, NULL);
+	TEST_ASSERT(!ret, "pthread_join() failed, ret=%d", ret);
+
+	kvm_vm_free(vm);
+	return 0;
+}
-- 
2.39.5 (Apple Git-154)

Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Wei-Lin Chang]

Hi Karl,

On Tue, Aug 04, 2026 at 12:44:04AM +0200, Karl Mehltretter wrote:
> A vCPU in L2 caches its shadow S2 MMU in vcpu->arch.hw_mmu.
> kvm_vcpu_init_nested() can grow kvm->arch.nested_mmus while initialising
> another vCPU: it copies the MMUs, publishes the new allocation, and frees
> the old one.  It updates pgt->mmu back-pointers, but not hw_mmu, leaving
> the first vCPU with a pointer to freed memory.  hw_mmu cannot be fixed up
> the same way: a running vCPU reads it without holding mmu_lock.  Copying
> also duplicates the MMU's refcount, leaving the live copy permanently
> elevated.
> 
> KASAN reports this as a slab-use-after-free in kvm_handle_guest_abort().
> 
> Make nested_mmus a pointer table and allocate each MMU separately.
> Growing the table now moves only pointer entries, preserving cached
> hw_mmu pointers, pgt->mmu back-pointers, and each MMU's refcount.  Fully
> initialise new MMUs before publishing the table and its size under
> mmu_lock.
> 
> The old failure path passed uninitialised entries to
> kvm_free_stage2_pgd(), which needs mmu->arch.  Use an allocation helper
> that returns only fully initialised MMUs, so error cleanup frees only
> completed objects; kvm_init_stage2_mmu() unwinds a failed initialisation.

Just a comment, instead of allocaing the nested mmus individually
another option is to just fail VCPU_INIT if any of the nested mmus have
refcnt > 0.

However, we probably want to return EBUSY in that case, but that's a
uAPI change.

Thanks,
Wei-Lin Chang

> 
> Fixes: 4f128f8e1aaa ("KVM: arm64: nv: Support multiple nested Stage-2 mmu structures")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 
> Tested on an arm64 KASAN kernel under QEMU TCG with EL2 emulation
> (-machine virt,virtualization=on -cpu max, kvm-arm.mode=nested): the
> selftest in patch 2 reports the slab-use-after-free without this patch
> and passes with it.
> 
>  arch/arm64/include/asm/kvm_host.h |  6 +-
>  arch/arm64/kvm/nested.c           | 92 +++++++++++++++++++++----------
>  2 files changed, 67 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index bae2c4f92ef5..f587b01039f9 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -319,10 +319,10 @@ struct kvm_arch {
>  	u64 fgu[__NR_FGT_GROUP_IDS__];
>  
>  	/*
> -	 * Stage 2 paging state for VMs with nested S2 using a virtual
> -	 * VMID.
> +	 * Stage 2 paging state for VMs with nested S2 using a virtual VMID.
> +	 * MMUs are individually allocated to keep their addresses stable.
>  	 */
> -	struct kvm_s2_mmu *nested_mmus;
> +	struct kvm_s2_mmu **nested_mmus;
>  	size_t nested_mmus_size;
>  	int nested_mmus_next;
>  
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index dfb96edbdc43..af804a5ddca7 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/err.h>
>  #include <linux/kvm.h>
>  #include <linux/kvm_host.h>
>  
> @@ -66,11 +67,36 @@ static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
>  	return kvm_init_stage2_mmu(kvm, mmu, kvm_get_pa_bits(kvm));
>  }
>  
> +static struct kvm_s2_mmu *alloc_nested_s2_mmu(struct kvm *kvm)
> +{
> +	struct kvm_s2_mmu *mmu;
> +	int ret;
> +
> +	mmu = kzalloc_obj(*mmu, GFP_KERNEL_ACCOUNT);
> +	if (!mmu)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = init_nested_s2_mmu(kvm, mmu);
> +	if (ret) {
> +		/* kvm_init_stage2_mmu() frees its internal allocations on error */
> +		kfree(mmu);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return mmu;
> +}
> +
> +static void free_nested_s2_mmu(struct kvm_s2_mmu *mmu)
> +{
> +	kvm_free_stage2_pgd(mmu);
> +	kfree(mmu);
> +}
> +
>  int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = vcpu->kvm;
> -	struct kvm_s2_mmu *tmp;
> -	int num_mmus, ret = 0;
> +	struct kvm_s2_mmu **tmp;
> +	int i, num_mmus, ret = 0;
>  
>  	if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features) &&
>  	    !cpus_have_final_cap(ARM64_HAS_HCR_NV1))
> @@ -96,38 +122,48 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>  		if (!tmp)
>  			return -ENOMEM;
>  
> +		/*
> +		 * Populate new slots before publishing: table walkers hold
> +		 * mmu_lock and iterate up to nested_mmus_size.
> +		 */
> +		for (i = kvm->arch.nested_mmus_size; i < num_mmus; i++) {
> +			struct kvm_s2_mmu *mmu = alloc_nested_s2_mmu(kvm);
> +
> +			if (IS_ERR(mmu)) {
> +				ret = PTR_ERR(mmu);
> +				break;
> +			}
> +
> +			tmp[i] = mmu;
> +		}
> +
> +		if (ret) {
> +			while (i-- > kvm->arch.nested_mmus_size)
> +				free_nested_s2_mmu(tmp[i]);
> +
> +			kvfree(tmp);
> +
> +			free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> +			vcpu->arch.ctxt.vncr_array = NULL;
> +
> +			return ret;
> +		}
> +
>  		write_lock(&kvm->mmu_lock);
>  
>  		if (kvm->arch.nested_mmus_size) {
>  			memcpy(tmp, kvm->arch.nested_mmus,
>  			       size_mul(sizeof(*tmp), kvm->arch.nested_mmus_size));
> -
> -			for (int i = 0; i < kvm->arch.nested_mmus_size; i++)
> -				tmp[i].pgt->mmu = &tmp[i];
>  		}
>  
>  		swap(kvm->arch.nested_mmus, tmp);
> +		kvm->arch.nested_mmus_size = num_mmus;
>  
>  		write_unlock(&kvm->mmu_lock);
>  
>  		kvfree(tmp);
>  	}
>  
> -	for (int i = kvm->arch.nested_mmus_size; !ret && i < num_mmus; i++)
> -		ret = init_nested_s2_mmu(kvm, &kvm->arch.nested_mmus[i]);
> -
> -	if (ret) {
> -		for (int i = kvm->arch.nested_mmus_size; i < num_mmus; i++)
> -			kvm_free_stage2_pgd(&kvm->arch.nested_mmus[i]);
> -
> -		free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> -		vcpu->arch.ctxt.vncr_array = NULL;
> -
> -		return ret;
> -	}
> -
> -	kvm->arch.nested_mmus_size = num_mmus;
> -
>  	return 0;
>  }
>  
> @@ -725,7 +761,7 @@ void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
>  	write_lock(&kvm->mmu_lock);
>  
>  	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (!kvm_s2_mmu_valid(mmu))
>  			continue;
> @@ -767,7 +803,7 @@ struct kvm_s2_mmu *lookup_s2_mmu(struct kvm_vcpu *vcpu)
>  	 *   if S2 translation is disabled.
>  	 */
>  	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (!kvm_s2_mmu_valid(mmu))
>  			continue;
> @@ -806,7 +842,7 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
>  	for (i = kvm->arch.nested_mmus_next;
>  	     i < (kvm->arch.nested_mmus_size + kvm->arch.nested_mmus_next);
>  	     i++) {
> -		s2_mmu = &kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
> +		s2_mmu = kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
>  
>  		if (atomic_read(&s2_mmu->refcnt) == 0)
>  			break;
> @@ -1223,7 +1259,7 @@ void kvm_nested_s2_wp(struct kvm *kvm)
>  		return;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (kvm_s2_mmu_valid(mmu))
>  			kvm_stage2_wp_range(mmu, 0, kvm_phys_size(mmu));
> @@ -1242,7 +1278,7 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
>  		return;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (kvm_s2_mmu_valid(mmu))
>  			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
> @@ -1261,7 +1297,7 @@ void kvm_nested_s2_flush(struct kvm *kvm)
>  		return;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (kvm_s2_mmu_valid(mmu))
>  			kvm_stage2_flush_range(mmu, 0, kvm_phys_size(mmu));
> @@ -1273,10 +1309,10 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
>  	int i;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (!WARN_ON(atomic_read(&mmu->refcnt)))
> -			kvm_free_stage2_pgd(mmu);
> +			free_nested_s2_mmu(mmu);
>  	}
>  	kvfree(kvm->arch.nested_mmus);
>  	kvm->arch.nested_mmus = NULL;
> 
> base-commit: 38436106b2f5ceb55950a7098c1a5804de2bde62
> -- 
> 2.39.5 (Apple Git-154)

Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Marc Zyngier]

On Mon, 03 Aug 2026 23:44:04 +0100,
Karl Mehltretter <kmehltretter@gmail.com> wrote:
> 
> A vCPU in L2 caches its shadow S2 MMU in vcpu->arch.hw_mmu.
> kvm_vcpu_init_nested() can grow kvm->arch.nested_mmus while initialising
> another vCPU: it copies the MMUs, publishes the new allocation, and frees
> the old one.  It updates pgt->mmu back-pointers, but not hw_mmu, leaving
> the first vCPU with a pointer to freed memory.  hw_mmu cannot be fixed up
> the same way: a running vCPU reads it without holding mmu_lock.  Copying
> also duplicates the MMU's refcount, leaving the live copy permanently
> elevated.
> 
> KASAN reports this as a slab-use-after-free in kvm_handle_guest_abort().
> 
> Make nested_mmus a pointer table and allocate each MMU separately.
> Growing the table now moves only pointer entries, preserving cached
> hw_mmu pointers, pgt->mmu back-pointers, and each MMU's refcount.  Fully
> initialise new MMUs before publishing the table and its size under
> mmu_lock.
> 
> The old failure path passed uninitialised entries to
> kvm_free_stage2_pgd(), which needs mmu->arch.  Use an allocation helper
> that returns only fully initialised MMUs, so error cleanup frees only
> completed objects; kvm_init_stage2_mmu() unwinds a failed initialisation.

I like the approach, but this is making things a bit more complicated
than they should really be IMO.

>
> Fixes: 4f128f8e1aaa ("KVM: arm64: nv: Support multiple nested Stage-2 mmu structures")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 
> Tested on an arm64 KASAN kernel under QEMU TCG with EL2 emulation
> (-machine virt,virtualization=on -cpu max, kvm-arm.mode=nested): the
> selftest in patch 2 reports the slab-use-after-free without this patch
> and passes with it.
> 
>  arch/arm64/include/asm/kvm_host.h |  6 +-
>  arch/arm64/kvm/nested.c           | 92 +++++++++++++++++++++----------
>  2 files changed, 67 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index bae2c4f92ef5..f587b01039f9 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -319,10 +319,10 @@ struct kvm_arch {
>  	u64 fgu[__NR_FGT_GROUP_IDS__];
>  
>  	/*
> -	 * Stage 2 paging state for VMs with nested S2 using a virtual
> -	 * VMID.
> +	 * Stage 2 paging state for VMs with nested S2 using a virtual VMID.
> +	 * MMUs are individually allocated to keep their addresses stable.
>  	 */
> -	struct kvm_s2_mmu *nested_mmus;
> +	struct kvm_s2_mmu **nested_mmus;
>  	size_t nested_mmus_size;
>  	int nested_mmus_next;
>  
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index dfb96edbdc43..af804a5ddca7 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/err.h>
>  #include <linux/kvm.h>
>  #include <linux/kvm_host.h>
>  
> @@ -66,11 +67,36 @@ static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
>  	return kvm_init_stage2_mmu(kvm, mmu, kvm_get_pa_bits(kvm));
>  }
>  
> +static struct kvm_s2_mmu *alloc_nested_s2_mmu(struct kvm *kvm)
> +{
> +	struct kvm_s2_mmu *mmu;
> +	int ret;
> +
> +	mmu = kzalloc_obj(*mmu, GFP_KERNEL_ACCOUNT);
> +	if (!mmu)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = init_nested_s2_mmu(kvm, mmu);
> +	if (ret) {
> +		/* kvm_init_stage2_mmu() frees its internal allocations on error */
> +		kfree(mmu);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return mmu;
> +}
> +
> +static void free_nested_s2_mmu(struct kvm_s2_mmu *mmu)
> +{
> +	kvm_free_stage2_pgd(mmu);
> +	kfree(mmu);
> +}
> +
>  int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = vcpu->kvm;
> -	struct kvm_s2_mmu *tmp;
> -	int num_mmus, ret = 0;
> +	struct kvm_s2_mmu **tmp;
> +	int i, num_mmus, ret = 0;
>  
>  	if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features) &&
>  	    !cpus_have_final_cap(ARM64_HAS_HCR_NV1))
> @@ -96,38 +122,48 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>  		if (!tmp)
>  			return -ENOMEM;
>  
> +		/*
> +		 * Populate new slots before publishing: table walkers hold
> +		 * mmu_lock and iterate up to nested_mmus_size.
> +		 */
> +		for (i = kvm->arch.nested_mmus_size; i < num_mmus; i++) {
> +			struct kvm_s2_mmu *mmu = alloc_nested_s2_mmu(kvm);
> +
> +			if (IS_ERR(mmu)) {
> +				ret = PTR_ERR(mmu);
> +				break;
> +			}
> +
> +			tmp[i] = mmu;
> +		}

You now are allocating each s2_mmu individually. But you know you want
at most S2_MMU_PER_VCPU structures, and not any extra ones. So you
could allocate one block for the current vcpu, and let the next guy
allocate its own quota.

The other thing is that reallocating the pointer array isn't great. It
adds complexity, and makes everything more fragile than it should be.

See the hack below that seems to work OK.

	M.

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 81d359ac7af14..65a1305c4a749 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -321,7 +321,7 @@ struct kvm_arch {
 	 * Stage 2 paging state for VMs with nested S2 using a virtual
 	 * VMID.
 	 */
-	struct kvm_s2_mmu *nested_mmus;
+	struct kvm_s2_mmu **nested_mmus;
 	size_t nested_mmus_size;
 	int nested_mmus_next;
 
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 012d711034d17..d21be647ac571 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -66,7 +66,7 @@ static inline u64 translate_ttbr0_el2_to_ttbr0_el1(u64 ttbr0)
 
 extern bool forward_smc_trap(struct kvm_vcpu *vcpu);
 extern bool forward_debug_exception(struct kvm_vcpu *vcpu);
-extern void kvm_init_nested(struct kvm *kvm);
+extern int kvm_init_nested(struct kvm *kvm);
 extern int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu);
 extern void kvm_init_nested_s2_mmu(struct kvm_s2_mmu *mmu);
 extern struct kvm_s2_mmu *lookup_s2_mmu(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9a6c72a186727..ae27ccc37b330 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -236,7 +236,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	mutex_unlock(&kvm->lock);
 #endif
 
-	kvm_init_nested(kvm);
+	ret = kvm_init_nested(kvm);
+	if (ret)
+		return ret;
 
 	ret = kvm_share_hyp(kvm, kvm + 1);
 	if (ret)
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dfb96edbdc43c..e7a066a41b552 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -44,11 +44,15 @@ struct vncr_tlb {
  */
 #define S2_MMU_PER_VCPU		2
 
-void kvm_init_nested(struct kvm *kvm)
+int kvm_init_nested(struct kvm *kvm)
 {
-	kvm->arch.nested_mmus = NULL;
+	kvm->arch.nested_mmus = kvmalloc_array(KVM_MAX_VCPUS * S2_MMU_PER_VCPU,
+					       sizeof(struct s2_mmu *),
+					       GFP_KERNEL_ACCOUNT);
 	kvm->arch.nested_mmus_size = 0;
 	atomic_set(&kvm->arch.vncr_map_count, 0);
+
+	return kvm->arch.nested_mmus ? 0 : -ENOMEM;
 }
 
 static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
@@ -69,8 +73,7 @@ static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
 int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = vcpu->kvm;
-	struct kvm_s2_mmu *tmp;
-	int num_mmus, ret = 0;
+	int num_mmus;
 
 	if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features) &&
 	    !cpus_have_final_cap(ARM64_HAS_HCR_NV1))
@@ -92,42 +95,34 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
 	num_mmus = atomic_read(&kvm->online_vcpus) * S2_MMU_PER_VCPU;
 
 	if (num_mmus > kvm->arch.nested_mmus_size) {
-		tmp = kvcalloc(num_mmus, sizeof(*tmp), GFP_KERNEL_ACCOUNT);
+		struct kvm_s2_mmu *tmp;
+		int ret = 0;
+
+		tmp = kvcalloc(S2_MMU_PER_VCPU, sizeof(*tmp), GFP_KERNEL_ACCOUNT);
 		if (!tmp)
 			return -ENOMEM;
 
-		write_lock(&kvm->mmu_lock);
+		for (int i = 0; !ret && i < S2_MMU_PER_VCPU; i++)
+			ret = init_nested_s2_mmu(kvm, &tmp[i]);
 
-		if (kvm->arch.nested_mmus_size) {
-			memcpy(tmp, kvm->arch.nested_mmus,
-			       size_mul(sizeof(*tmp), kvm->arch.nested_mmus_size));
+		if (ret) {
+			for (int i = 0; i < S2_MMU_PER_VCPU; i++)
+				kvm_free_stage2_pgd(&tmp[i]);
 
-			for (int i = 0; i < kvm->arch.nested_mmus_size; i++)
-				tmp[i].pgt->mmu = &tmp[i];
+			kvfree(tmp);
+			free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
+			vcpu->arch.ctxt.vncr_array = NULL;
+			return ret;
 		}
+		
+		guard(write_lock)(&kvm->mmu_lock);
 
-		swap(kvm->arch.nested_mmus, tmp);
+		for (int i = 0; i < S2_MMU_PER_VCPU; i++)
+			kvm->arch.nested_mmus[i + kvm->arch.nested_mmus_size] = &tmp[i];
 
-		write_unlock(&kvm->mmu_lock);
-
-		kvfree(tmp);
+		kvm->arch.nested_mmus_size += S2_MMU_PER_VCPU;
 	}
 
-	for (int i = kvm->arch.nested_mmus_size; !ret && i < num_mmus; i++)
-		ret = init_nested_s2_mmu(kvm, &kvm->arch.nested_mmus[i]);
-
-	if (ret) {
-		for (int i = kvm->arch.nested_mmus_size; i < num_mmus; i++)
-			kvm_free_stage2_pgd(&kvm->arch.nested_mmus[i]);
-
-		free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
-		vcpu->arch.ctxt.vncr_array = NULL;
-
-		return ret;
-	}
-
-	kvm->arch.nested_mmus_size = num_mmus;
-
 	return 0;
 }
 
@@ -725,7 +720,7 @@ void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
 	write_lock(&kvm->mmu_lock);
 
 	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (!kvm_s2_mmu_valid(mmu))
 			continue;
@@ -767,7 +762,7 @@ struct kvm_s2_mmu *lookup_s2_mmu(struct kvm_vcpu *vcpu)
 	 *   if S2 translation is disabled.
 	 */
 	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (!kvm_s2_mmu_valid(mmu))
 			continue;
@@ -806,7 +801,7 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
 	for (i = kvm->arch.nested_mmus_next;
 	     i < (kvm->arch.nested_mmus_size + kvm->arch.nested_mmus_next);
 	     i++) {
-		s2_mmu = &kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
+		s2_mmu = kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
 
 		if (atomic_read(&s2_mmu->refcnt) == 0)
 			break;
@@ -1223,7 +1218,7 @@ void kvm_nested_s2_wp(struct kvm *kvm)
 		return;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
 			kvm_stage2_wp_range(mmu, 0, kvm_phys_size(mmu));
@@ -1242,7 +1237,7 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
 		return;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
 			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
@@ -1261,7 +1256,7 @@ void kvm_nested_s2_flush(struct kvm *kvm)
 		return;
 
 	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (kvm_s2_mmu_valid(mmu))
 			kvm_stage2_flush_range(mmu, 0, kvm_phys_size(mmu));
@@ -1270,13 +1265,14 @@ void kvm_nested_s2_flush(struct kvm *kvm)
 
 void kvm_arch_flush_shadow_all(struct kvm *kvm)
 {
-	int i;
-
-	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
-		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
+	for (int i = kvm->arch.nested_mmus_size - 1; i >= 0; i--) {
+		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
 
 		if (!WARN_ON(atomic_read(&mmu->refcnt)))
 			kvm_free_stage2_pgd(mmu);
+
+		if ((i % S2_MMU_PER_VCPU) == 0)
+			kvfree(mmu);
 	}
 	kvfree(kvm->arch.nested_mmus);
 	kvm->arch.nested_mmus = NULL;

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 2/2] KVM: arm64: selftests: Add a nested S2 MMU realloc test

[Marc Zyngier]

On Mon, 03 Aug 2026 23:44:05 +0100,
Karl Mehltretter <kmehltretter@gmail.com> wrote:
> 
> Add a regression test for a stale vcpu->arch.hw_mmu reference when
> initialising a vCPU grows the nested S2 MMU table.
> 
> The test drives vCPU0 into L2 through a minimal L1 stage-2 identity map,
> pins it to a second pCPU where it spins in L2, and then initialises
> vCPU1.  That initialisation grows the nested MMU table while vCPU0 still
> holds one of its entries; keeping vCPU0 on a pCPU of its own means the
> reference stays live without relying on hw_mmu being retained across a
> schedule-out.  vCPU0 is then released and has to run to completion.
> 
> Creating vCPU1 up front is what allows the in-kernel VGIC to be used:
> kvm_arch_vcpu_precreate() refuses KVM_CREATE_VCPU once the VGIC has been
> initialised, which the test does before its first KVM_RUN.  Creation on
> its own increments online_vcpus, so deferring vCPU1's KVM_ARM_VCPU_INIT
> until vCPU0 is in L2 still grows the table.
> 
> With KASAN enabled, an unfixed kernel reports a slab-use-after-free in
> kvm_handle_guest_abort(); with the fix it completes cleanly.

The problem is that we can't mandate selftests to rely on KASAN on the
host. Selftests are there to verify that we match the architecture
requirements.

If anything, this is a nice hack to demonstrate the problem (and yes,
it fires here).

> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 
> The test requires nested virtualization and two pCPUs.  Under QEMU TCG
> it takes ~233s, exceeding the 120s timeout in
> tools/testing/selftests/kvm/settings; psci_test takes ~167s in the same
> boot, so emulating the second vCPU is the dominant cost.

# time /host/home/maz/nested_mmu_realloc_test 
Random seed: 0x6b8b4567
Running control thread on pCPU 0, vCPU thread on pCPU 1
vCPU0 is running in L2; initializing vCPU1 to grow the table
vCPU1 initialized; releasing vCPU0
vCPU0 resumed after nested MMU resize

real	0m0.161s
user	0m0.007s
sys	0m0.109s

This is with KVM running as an L1 already...

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Marc Zyngier]

[+ Shen, who privately reported this with a 7 hour lead...]

On Tue, 04 Aug 2026 15:31:13 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Mon, 03 Aug 2026 23:44:04 +0100,
> Karl Mehltretter <kmehltretter@gmail.com> wrote:
> > 
> > A vCPU in L2 caches its shadow S2 MMU in vcpu->arch.hw_mmu.
> > kvm_vcpu_init_nested() can grow kvm->arch.nested_mmus while initialising
> > another vCPU: it copies the MMUs, publishes the new allocation, and frees
> > the old one.  It updates pgt->mmu back-pointers, but not hw_mmu, leaving
> > the first vCPU with a pointer to freed memory.  hw_mmu cannot be fixed up
> > the same way: a running vCPU reads it without holding mmu_lock.  Copying
> > also duplicates the MMU's refcount, leaving the live copy permanently
> > elevated.
> > 
> > KASAN reports this as a slab-use-after-free in kvm_handle_guest_abort().
> > 
> > Make nested_mmus a pointer table and allocate each MMU separately.
> > Growing the table now moves only pointer entries, preserving cached
> > hw_mmu pointers, pgt->mmu back-pointers, and each MMU's refcount.  Fully
> > initialise new MMUs before publishing the table and its size under
> > mmu_lock.
> > 
> > The old failure path passed uninitialised entries to
> > kvm_free_stage2_pgd(), which needs mmu->arch.  Use an allocation helper
> > that returns only fully initialised MMUs, so error cleanup frees only
> > completed objects; kvm_init_stage2_mmu() unwinds a failed initialisation.
> 
> I like the approach, but this is making things a bit more complicated
> than they should really be IMO.
> 
> >
> > Fixes: 4f128f8e1aaa ("KVM: arm64: nv: Support multiple nested Stage-2 mmu structures")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-fable-5
> > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> > ---
> > 
> > Tested on an arm64 KASAN kernel under QEMU TCG with EL2 emulation
> > (-machine virt,virtualization=on -cpu max, kvm-arm.mode=nested): the
> > selftest in patch 2 reports the slab-use-after-free without this patch
> > and passes with it.
> > 
> >  arch/arm64/include/asm/kvm_host.h |  6 +-
> >  arch/arm64/kvm/nested.c           | 92 +++++++++++++++++++++----------
> >  2 files changed, 67 insertions(+), 31 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index bae2c4f92ef5..f587b01039f9 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -319,10 +319,10 @@ struct kvm_arch {
> >  	u64 fgu[__NR_FGT_GROUP_IDS__];
> >  
> >  	/*
> > -	 * Stage 2 paging state for VMs with nested S2 using a virtual
> > -	 * VMID.
> > +	 * Stage 2 paging state for VMs with nested S2 using a virtual VMID.
> > +	 * MMUs are individually allocated to keep their addresses stable.
> >  	 */
> > -	struct kvm_s2_mmu *nested_mmus;
> > +	struct kvm_s2_mmu **nested_mmus;
> >  	size_t nested_mmus_size;
> >  	int nested_mmus_next;
> >  
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index dfb96edbdc43..af804a5ddca7 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> > @@ -5,6 +5,7 @@
> >   */
> >  
> >  #include <linux/bitfield.h>
> > +#include <linux/err.h>
> >  #include <linux/kvm.h>
> >  #include <linux/kvm_host.h>
> >  
> > @@ -66,11 +67,36 @@ static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
> >  	return kvm_init_stage2_mmu(kvm, mmu, kvm_get_pa_bits(kvm));
> >  }
> >  
> > +static struct kvm_s2_mmu *alloc_nested_s2_mmu(struct kvm *kvm)
> > +{
> > +	struct kvm_s2_mmu *mmu;
> > +	int ret;
> > +
> > +	mmu = kzalloc_obj(*mmu, GFP_KERNEL_ACCOUNT);
> > +	if (!mmu)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	ret = init_nested_s2_mmu(kvm, mmu);
> > +	if (ret) {
> > +		/* kvm_init_stage2_mmu() frees its internal allocations on error */
> > +		kfree(mmu);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	return mmu;
> > +}
> > +
> > +static void free_nested_s2_mmu(struct kvm_s2_mmu *mmu)
> > +{
> > +	kvm_free_stage2_pgd(mmu);
> > +	kfree(mmu);
> > +}
> > +
> >  int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
> >  {
> >  	struct kvm *kvm = vcpu->kvm;
> > -	struct kvm_s2_mmu *tmp;
> > -	int num_mmus, ret = 0;
> > +	struct kvm_s2_mmu **tmp;
> > +	int i, num_mmus, ret = 0;
> >  
> >  	if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features) &&
> >  	    !cpus_have_final_cap(ARM64_HAS_HCR_NV1))
> > @@ -96,38 +122,48 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
> >  		if (!tmp)
> >  			return -ENOMEM;
> >  
> > +		/*
> > +		 * Populate new slots before publishing: table walkers hold
> > +		 * mmu_lock and iterate up to nested_mmus_size.
> > +		 */
> > +		for (i = kvm->arch.nested_mmus_size; i < num_mmus; i++) {
> > +			struct kvm_s2_mmu *mmu = alloc_nested_s2_mmu(kvm);
> > +
> > +			if (IS_ERR(mmu)) {
> > +				ret = PTR_ERR(mmu);
> > +				break;
> > +			}
> > +
> > +			tmp[i] = mmu;
> > +		}
> 
> You now are allocating each s2_mmu individually. But you know you want
> at most S2_MMU_PER_VCPU structures, and not any extra ones. So you
> could allocate one block for the current vcpu, and let the next guy
> allocate its own quota.
> 
> The other thing is that reallocating the pointer array isn't great. It
> adds complexity, and makes everything more fragile than it should be.
> 
> See the hack below that seems to work OK.
> 
> 	M.
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 81d359ac7af14..65a1305c4a749 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -321,7 +321,7 @@ struct kvm_arch {
>  	 * Stage 2 paging state for VMs with nested S2 using a virtual
>  	 * VMID.
>  	 */
> -	struct kvm_s2_mmu *nested_mmus;
> +	struct kvm_s2_mmu **nested_mmus;
>  	size_t nested_mmus_size;
>  	int nested_mmus_next;
>  
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 012d711034d17..d21be647ac571 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -66,7 +66,7 @@ static inline u64 translate_ttbr0_el2_to_ttbr0_el1(u64 ttbr0)
>  
>  extern bool forward_smc_trap(struct kvm_vcpu *vcpu);
>  extern bool forward_debug_exception(struct kvm_vcpu *vcpu);
> -extern void kvm_init_nested(struct kvm *kvm);
> +extern int kvm_init_nested(struct kvm *kvm);
>  extern int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu);
>  extern void kvm_init_nested_s2_mmu(struct kvm_s2_mmu *mmu);
>  extern struct kvm_s2_mmu *lookup_s2_mmu(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9a6c72a186727..ae27ccc37b330 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -236,7 +236,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	mutex_unlock(&kvm->lock);
>  #endif
>  
> -	kvm_init_nested(kvm);
> +	ret = kvm_init_nested(kvm);
> +	if (ret)
> +		return ret;
>  
>  	ret = kvm_share_hyp(kvm, kvm + 1);
>  	if (ret)
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index dfb96edbdc43c..e7a066a41b552 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -44,11 +44,15 @@ struct vncr_tlb {
>   */
>  #define S2_MMU_PER_VCPU		2
>  
> -void kvm_init_nested(struct kvm *kvm)
> +int kvm_init_nested(struct kvm *kvm)
>  {
> -	kvm->arch.nested_mmus = NULL;
> +	kvm->arch.nested_mmus = kvmalloc_array(KVM_MAX_VCPUS * S2_MMU_PER_VCPU,
> +					       sizeof(struct s2_mmu *),
> +					       GFP_KERNEL_ACCOUNT);
>  	kvm->arch.nested_mmus_size = 0;
>  	atomic_set(&kvm->arch.vncr_map_count, 0);
> +
> +	return kvm->arch.nested_mmus ? 0 : -ENOMEM;
>  }
>  
>  static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
> @@ -69,8 +73,7 @@ static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
>  int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = vcpu->kvm;
> -	struct kvm_s2_mmu *tmp;
> -	int num_mmus, ret = 0;
> +	int num_mmus;
>  
>  	if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features) &&
>  	    !cpus_have_final_cap(ARM64_HAS_HCR_NV1))
> @@ -92,42 +95,34 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
>  	num_mmus = atomic_read(&kvm->online_vcpus) * S2_MMU_PER_VCPU;
>  
>  	if (num_mmus > kvm->arch.nested_mmus_size) {
> -		tmp = kvcalloc(num_mmus, sizeof(*tmp), GFP_KERNEL_ACCOUNT);
> +		struct kvm_s2_mmu *tmp;
> +		int ret = 0;
> +
> +		tmp = kvcalloc(S2_MMU_PER_VCPU, sizeof(*tmp), GFP_KERNEL_ACCOUNT);
>  		if (!tmp)
>  			return -ENOMEM;
>  
> -		write_lock(&kvm->mmu_lock);
> +		for (int i = 0; !ret && i < S2_MMU_PER_VCPU; i++)
> +			ret = init_nested_s2_mmu(kvm, &tmp[i]);
>  
> -		if (kvm->arch.nested_mmus_size) {
> -			memcpy(tmp, kvm->arch.nested_mmus,
> -			       size_mul(sizeof(*tmp), kvm->arch.nested_mmus_size));
> +		if (ret) {
> +			for (int i = 0; i < S2_MMU_PER_VCPU; i++)
> +				kvm_free_stage2_pgd(&tmp[i]);
>  
> -			for (int i = 0; i < kvm->arch.nested_mmus_size; i++)
> -				tmp[i].pgt->mmu = &tmp[i];
> +			kvfree(tmp);
> +			free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> +			vcpu->arch.ctxt.vncr_array = NULL;
> +			return ret;
>  		}
> +		
> +		guard(write_lock)(&kvm->mmu_lock);
>  
> -		swap(kvm->arch.nested_mmus, tmp);
> +		for (int i = 0; i < S2_MMU_PER_VCPU; i++)
> +			kvm->arch.nested_mmus[i + kvm->arch.nested_mmus_size] = &tmp[i];
>  
> -		write_unlock(&kvm->mmu_lock);
> -
> -		kvfree(tmp);
> +		kvm->arch.nested_mmus_size += S2_MMU_PER_VCPU;
>  	}
>  
> -	for (int i = kvm->arch.nested_mmus_size; !ret && i < num_mmus; i++)
> -		ret = init_nested_s2_mmu(kvm, &kvm->arch.nested_mmus[i]);
> -
> -	if (ret) {
> -		for (int i = kvm->arch.nested_mmus_size; i < num_mmus; i++)
> -			kvm_free_stage2_pgd(&kvm->arch.nested_mmus[i]);
> -
> -		free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> -		vcpu->arch.ctxt.vncr_array = NULL;
> -
> -		return ret;
> -	}
> -
> -	kvm->arch.nested_mmus_size = num_mmus;
> -
>  	return 0;
>  }
>  
> @@ -725,7 +720,7 @@ void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
>  	write_lock(&kvm->mmu_lock);
>  
>  	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (!kvm_s2_mmu_valid(mmu))
>  			continue;
> @@ -767,7 +762,7 @@ struct kvm_s2_mmu *lookup_s2_mmu(struct kvm_vcpu *vcpu)
>  	 *   if S2 translation is disabled.
>  	 */
>  	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (!kvm_s2_mmu_valid(mmu))
>  			continue;
> @@ -806,7 +801,7 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
>  	for (i = kvm->arch.nested_mmus_next;
>  	     i < (kvm->arch.nested_mmus_size + kvm->arch.nested_mmus_next);
>  	     i++) {
> -		s2_mmu = &kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
> +		s2_mmu = kvm->arch.nested_mmus[i % kvm->arch.nested_mmus_size];
>  
>  		if (atomic_read(&s2_mmu->refcnt) == 0)
>  			break;
> @@ -1223,7 +1218,7 @@ void kvm_nested_s2_wp(struct kvm *kvm)
>  		return;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (kvm_s2_mmu_valid(mmu))
>  			kvm_stage2_wp_range(mmu, 0, kvm_phys_size(mmu));
> @@ -1242,7 +1237,7 @@ void kvm_nested_s2_unmap(struct kvm *kvm, bool may_block)
>  		return;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (kvm_s2_mmu_valid(mmu))
>  			kvm_stage2_unmap_range(mmu, 0, kvm_phys_size(mmu), may_block);
> @@ -1261,7 +1256,7 @@ void kvm_nested_s2_flush(struct kvm *kvm)
>  		return;
>  
>  	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (kvm_s2_mmu_valid(mmu))
>  			kvm_stage2_flush_range(mmu, 0, kvm_phys_size(mmu));
> @@ -1270,13 +1265,14 @@ void kvm_nested_s2_flush(struct kvm *kvm)
>  
>  void kvm_arch_flush_shadow_all(struct kvm *kvm)
>  {
> -	int i;
> -
> -	for (i = 0; i < kvm->arch.nested_mmus_size; i++) {
> -		struct kvm_s2_mmu *mmu = &kvm->arch.nested_mmus[i];
> +	for (int i = kvm->arch.nested_mmus_size - 1; i >= 0; i--) {
> +		struct kvm_s2_mmu *mmu = kvm->arch.nested_mmus[i];
>  
>  		if (!WARN_ON(atomic_read(&mmu->refcnt)))
>  			kvm_free_stage2_pgd(mmu);
> +
> +		if ((i % S2_MMU_PER_VCPU) == 0)
> +			kvfree(mmu);
>  	}
>  	kvfree(kvm->arch.nested_mmus);
>  	kvm->arch.nested_mmus = NULL;
> 
> -- 
> Without deviation from the norm, progress is not possible.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Karl Mehltretter]

On Tue, Aug 04, 2026 at 03:56:47PM +0100, Marc Zyngier wrote:
> > The other thing is that reallocating the pointer array isn't great. It
> > adds complexity, and makes everything more fragile than it should be.
> > 
> > See the hack below that seems to work OK.
> > 

Your draft is indeed a bit simpler.

Should I send a cleaned-up version as v2?

I would allocate the table on the first call to kvm_vcpu_init_nested()
and not in kvm_init_nested(), so it cannot leak if VM creation
fails. 

There's another issue in the current code: the ptdump debugfs file
keeps the raw mmu pointer as its private data. I would add that to the
commit message.

Thanks,
Karl

Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Marc Zyngier]

On Tue, 04 Aug 2026 22:54:30 +0100,
Karl Mehltretter <kmehltretter@gmail.com> wrote:
> 
> On Tue, Aug 04, 2026 at 03:56:47PM +0100, Marc Zyngier wrote:
> > > The other thing is that reallocating the pointer array isn't great. It
> > > adds complexity, and makes everything more fragile than it should be.
> > > 
> > > See the hack below that seems to work OK.
> > > 
> 
> Your draft is indeed a bit simpler.
> 
> Should I send a cleaned-up version as v2?

Yes, please.

> 
> I would allocate the table on the first call to kvm_vcpu_init_nested()
> and not in kvm_init_nested(), so it cannot leak if VM creation
> fails.

I'd rather keep it on the VM creation path. This is VM-wide data, by
definition, and given that its size doesn't depend on the number of
vcpus anymore (we allocate the maximum once and for all), it is right
where it belongs. See the untested hack below for the freeing on error.

> There's another issue in the current code: the ptdump debugfs file
> keeps the raw mmu pointer as its private data. I would add that to the
> commit message.

Why is that a problem? With this approach, the mmu pointers are always
expected to be valid, irrespective of the allocation pattern, and we
only publish pointers to the dumper when the S2_mmu is actively being
used.

We're about to rip out the NV ptdump anyway as it has many other
problems, but I'd like to understand what you see that I don't.

Thanks,

	M.

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9e42e92dbc7b1..e883e45382fb2 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -223,10 +223,6 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	mutex_unlock(&kvm->lock);
 #endif
 
-	ret = kvm_init_nested(kvm);
-	if (ret)
-		return ret;
-
 	ret = kvm_share_hyp(kvm, kvm + 1);
 	if (ret)
 		return ret;
@@ -241,6 +237,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	if (ret)
 		goto err_free_cpumask;
 
+	ret = kvm_init_nested(kvm);
+	if (ret)
+		goto err_uninit_mmu;
+
 	if (is_protected_kvm_enabled()) {
 		/*
 		 * If any failures occur after this is successful, make sure to
@@ -269,6 +269,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 err_uninit_mmu:
 	kvm_uninit_stage2_mmu(kvm);
+	kvfree(kvm->arch.nested_mmus);
 err_free_cpumask:
 	free_cpumask_var(kvm->arch.supported_cpus);
 err_unshare_kvm:

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

[Karl Mehltretter]

On Wed, Aug 05, 2026 at 08:32:29AM +0100, Marc Zyngier wrote:
> I'd rather keep it on the VM creation path. This is VM-wide data, by
> definition, and given that its size doesn't depend on the number of
> vcpus anymore (we allocate the maximum once and for all), it is right
> where it belongs. See the untested hack below for the freeing on error.

I'll send a v2 with this soon.

> 
> > There's another issue in the current code: the ptdump debugfs file
> > keeps the raw mmu pointer as its private data. I would add that to the
> > commit message.
> 
> Why is that a problem? With this approach, the mmu pointers are always
> expected to be valid, irrespective of the allocation pattern, and we
> only publish pointers to the dumper when the S2_mmu is actively being
> used.
> 

Sorry I was talking about the old implementation. Array reallocation
leaves ptdump i_private pointers dangling.

I'm dropping the selftest for v2. It was a useful KASAN trigger, but is 
not a good fit for the current suite.

Thanks,
Karl