Re: [PATCH] kvm: x86: Do proper cleanup if kvm_x86_ops->vm_init() fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/29/22 07:36, Sean Christopherson wrote:
On Thu, Jul 28, 2022, Junaid Shahid wrote:
If vm_init() fails [which can happen, for instance, if a memory
allocation fails during avic_vm_init()], we need to cleanup some
state in order to avoid resource leaks.

Signed-off-by: Junaid Shahid <junaids@xxxxxxxxxx>
---
  arch/x86/kvm/x86.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f389691d8c04..ef5fd2f05c79 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12064,8 +12064,14 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
  	kvm_hv_init_vm(kvm);
  	kvm_xen_init_vm(kvm);
- return static_call(kvm_x86_vm_init)(kvm);
+	ret = static_call(kvm_x86_vm_init)(kvm);
+	if (ret)
+		goto out_uninit_mmu;
+ return 0;
+
+out_uninit_mmu:
+	kvm_mmu_uninit_vm(kvm);

Hrm, this works for now (I think), but I really don't like that kvm_apicv_init(),
kvm_hv_init_vm(), and kvm_xen_init_vm() all do something without that something
being unwound on failure.  E.g. both Hyper-V and Xen have a paired "destroy"
function, it just so happens that their destroy paths are guaranteed nops in this
case.

AFAICT, there are no dependencies on doing vendor init at the end, so what if we
hoist it up so that all paths that can fail are at the top?


Ack. I'll move the vendor init call up. I think I'll skip breaking out a separate kvm_x86_op for apicv_init() for now.

Thanks,
Junaid



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux