linux-kernel - [PATCH] KVM: Fix error path in kvm_vm_ioctl_create_vcpu() on xa

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240730155646.1687-1-will@kernel.org>
Date: Tue, 30 Jul 2024 16:56:46 +0100
From: Will Deacon <will@...nel.org>
To: kvm@...r.kernel.org
Cc: linux-kernel@...r.kernel.org,
	Will Deacon <will@...nel.org>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Michal Luczaj <mhal@...x.co>,
	Alexander Potapenko <glider@...gle.com>,
	Marc Zyngier <maz@...nel.org>
Subject: [PATCH] KVM: Fix error path in kvm_vm_ioctl_create_vcpu() on xa_store() failure

If the xa_store() fails in kvm_vm_ioctl_create_vcpu() then we shouldn't
drop the reference to the 'struct kvm' because the vCPU fd has been
installed and will take care of the refcounting.

This was found by inspection, but forcing the xa_store() to fail
confirms the problem:

 | Unable to handle kernel paging request at virtual address ffff800080ecd960
 | Call trace:
 |  _raw_spin_lock_irq+0x2c/0x70
 |  kvm_irqfd_release+0x24/0xa0
 |  kvm_vm_release+0x1c/0x38
 |  __fput+0x88/0x2ec
 |  ____fput+0x10/0x1c
 |  task_work_run+0xb0/0xd4
 |  do_exit+0x210/0x854
 |  do_group_exit+0x70/0x98
 |  get_signal+0x6b0/0x73c
 |  do_signal+0xa4/0x11e8
 |  do_notify_resume+0x60/0x12c
 |  el0_svc+0x64/0x68
 |  el0t_64_sync_handler+0x84/0xfc
 |  el0t_64_sync+0x190/0x194
 | Code: b9000909 d503201f 2a1f03e1 52800028 (88e17c08)

Add a new label to the error path so that we can branch directly to the
xa_release() if the xa_store() fails.

Cc: Paolo Bonzini <pbonzini@...hat.com>
Cc: Michal Luczaj <mhal@...x.co>
Cc: Alexander Potapenko <glider@...gle.com>
Cc: Marc Zyngier <maz@...nel.org>
Signed-off-by: Will Deacon <will@...nel.org>
---
 virt/kvm/kvm_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d0788d0a72cc..b80dd8cead8c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4293,7 +4293,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
 
 	if (KVM_BUG_ON(xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
 		r = -EINVAL;
-		goto kvm_put_xa_release;
+		goto err_xa_release;
 	}
 
 	/*
@@ -4310,6 +4310,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
 
 kvm_put_xa_release:
 	kvm_put_kvm_no_destroy(kvm);
+err_xa_release:
 	xa_release(&kvm->vcpu_array, vcpu->vcpu_idx);
 unlock_vcpu_destroy:
 	mutex_unlock(&kvm->lock);
-- 
2.46.0.rc1.232.g9752f9e123-goog