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

[Karl Mehltretter <kmehltretter@gmail.com>]

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)