On 10/23/19 11:21 AM, Sean Christopherson wrote: >> out_err_no_disable: >> refcount_set(&kvm->users_count, 0); >> + kvm_arch_destroy_vm(kvm); > > Calling destroy_vm() after zeroing the refcount could lead to a refcount > underrun (and a WARN with CONFIG_REFCOUNT_FULL=y) if an arch were to do > kvm_put_kvm() in destroy_vm() to pair with a kvm_get_kvm() in create_vm(). > I doubt any arch actually does that, but it's technically possible since > kvm_arch_create_vm() is called with users_count=1. > > If we wanted to be paranoid, a follow-up patch could change refcount_set() > to WARN_ON(!refcount_dec_and_dest()), e.g.: > > kvm_arch_destroy_vm(kvm); > WARN_ON(!refcount_dec_and_dest(&kvm->users_count)); > AFAICT the kvm->users_count is already 0 before kvm_arch_destroy_vm() is called from kvm_destroy_vm() in the normal case. So there really shouldn't be any arch that does a kvm_put_kvm() inside kvm_arch_destroy_vm(). I think it might be better to keep the kvm_arch_destroy_vm() call after the refcount_set() to be consistent with the normal path.
