[PATCH 08/16] KVM: planes: Publish a new plane only once KVM_CREATE_PLANE cannot fail

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

kvm_create_plane() publishes the new plane in kvm->planes[] before
kvm_vm_ioctl_create_plane() can still fail at anon_inode_getfile(),
and the error path freed the plane without unpublishing it. Anything
that later reads kvm->planes[] reaches the freed plane, and VM teardown
destroys it a second time.

Unpublishing it on the error path is not enough either. Some readers of
kvm->planes[] take no lock: the SEV VMPL-switch target lookup takes the
level from the guest, so it can pick up a plane that is published and
then freed with no grace period in between.

Publish the plane in kvm_vm_ioctl_create_plane() only after
anon_inode_getfile() succeeds, so a failing create frees a plane that
no reader has ever seen. kvm_create_vm() publishes plane 0 itself,
before the VM is reachable. The store stays smp_store_release(), and
the lock-free readers (MSI delivery and the SEV VMPL target lookup) now
load the pointer with READ_ONCE() to pair with it.

Found by a local Sashiko review.

Fixes the error path in "kvm: Implement KVM_CREATE_PLANE ioctl".

Fixes: https://lore.kernel.org/all/20260608144252.351443-30-joro@xxxxxxxxxx/
Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
 arch/x86/kvm/irq.c     |  5 +++--
 arch/x86/kvm/svm/sev.c |  2 +-
 virt/kvm/kvm_main.c    | 16 ++++++++++++----
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index f3044aec7188..26912161997e 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -235,7 +235,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 		return -1;
 
 	kvm_msi_to_lapic_irq(kvm, e, &irq);
-	plane = kvm->planes[e->msi.plane_level];
+	plane = READ_ONCE(kvm->planes[e->msi.plane_level]);
 
 	return kvm_irq_delivery_to_apic(plane, NULL, &irq);
 }
@@ -260,7 +260,8 @@ int kvm_arch_set_irq_inatomic(struct kvm_kernel_irq_routing_entry *e,
 
 		kvm_msi_to_lapic_irq(kvm, e, &irq);
 
-		if (kvm_irq_delivery_to_apic_fast(kvm->planes[e->msi.plane_level], NULL, &irq, &r))
+		if (kvm_irq_delivery_to_apic_fast(READ_ONCE(kvm->planes[e->msi.plane_level]),
+						  NULL, &irq, &r))
 			return r;
 		break;
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index fd7605e47216..32a4ba4ffdbb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4249,7 +4249,7 @@ static int sev_snp_ap_creation(struct kvm_vcpu *vcpu)
 	vmpl = array_index_nospec(vmpl, SVM_SEV_VMPL_MAX);
 
 	/* Obtain the target plane and vCPU */
-	target_plane = vcpu->kvm->planes[vmpl];
+	target_plane = READ_ONCE(vcpu->kvm->planes[vmpl]);
 	if (target_plane) {
 		target_vcpu = plane_get_vcpu(target_plane, apic_id);
 	} else {
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 95de904f0c61..d256bbf301f4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1252,8 +1252,7 @@ static struct kvm_plane *kvm_create_plane(struct kvm *kvm, unsigned plane_level)
 	if (kvm_arch_plane_init(kvm, plane, plane_level))
 		goto out_free_plane;
 
-	kvm->planes[plane_level] = plane;
-
+	/* The caller publishes the plane in kvm->planes[]. */
 	return plane;
 
 out_free_plane:
@@ -1312,7 +1311,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 	BUILD_BUG_ON(KVM_MEM_SLOTS_NUM > SHRT_MAX);
 
 	/* Initialize planes array and allocate plane 0 */
-	if (kvm_create_plane(kvm, 0) == NULL) {
+	kvm->planes[0] = kvm_create_plane(kvm, 0);
+	if (!kvm->planes[0]) {
 		r = -ENOMEM;
 		goto out_no_planes;
 	}
@@ -5627,13 +5627,21 @@ static int kvm_vm_ioctl_create_plane(struct kvm *kvm, unsigned id)
 		goto put_kvm;
 	}
 
+	/*
+	 * Publish only once nothing can fail, so that the error path frees
+	 * a plane no reader has seen.  Pairs with the READ_ONCE() of the
+	 * lock-free readers of kvm->planes[] (MSI delivery, SEV VMPL
+	 * targeting): the plane's fields must be visible before the
+	 * pointer is.
+	 */
+	smp_store_release(&kvm->planes[id], plane);
 	kvm->has_planes = true;
 	fd_install(fd, file);
 	return fd;
 
 put_kvm:
-	kvm_put_kvm(kvm);
 	kvm_destroy_one_plane(plane);
+	kvm_put_kvm(kvm);
 put_fd:
 	put_unused_fd(fd);
 	return r;
-- 
2.50.1