On Fri, Oct 25, 2019 at 01:37:56PM +0200, Paolo Bonzini wrote:
> On 25/10/19 01:29, Sean Christopherson wrote:
> > On Thu, Oct 24, 2019 at 04:03:27PM -0700, Jim Mattson wrote:
> >> From: John Sperbeck <jsperbeck@xxxxxxxxxx>
> >>
> >> In kvm_create_vm(), if we've successfully called kvm_arch_init_vm(), but
> >> then fail later in the function, we need to call kvm_arch_destroy_vm()
> >> so that it can do any necessary cleanup (like freeing memory).
> >>
> >> Fixes: 44a95dae1d229a ("KVM: x86: Detect and Initialize AVIC support")
> >>
> >> Signed-off-by: John Sperbeck <jsperbeck@xxxxxxxxxx>
> >> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> >> Reviewed-by: Junaid Shahid <junaids@xxxxxxxxxx>
> >> ---
> >> virt/kvm/kvm_main.c | 10 ++++++----
> >> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> Sorry for the back and forth on this---I actually preferred the version
> that did not move refcount_set. It seems to me that kvm_get_kvm() in
> kvm_arch_init_vm() should be okay as long as it is balanced in
> kvm_arch_destroy_vm(). So we can apply patch 2 first, and then:
No, this will effectively leak the VM because you'll end up with a cyclical
reference to kvm_put_kvm(), i.e. users_count will never hit zero.
void kvm_put_kvm(struct kvm *kvm)
{
if (refcount_dec_and_test(&kvm->users_count))
kvm_destroy_vm(kvm);
|
-> kvm_arch_destroy_vm()
|
-> kvm_put_kvm()
}
