Currently it is possible that the final put of a KVM reference comes from
vfio during its device close operation.  This occurs while the vfio group
lock is held; however, if the vfio device is still in the kvm device list,
then the following call chain could result in a deadlock:

VFIO holds group->group_lock/group_rwsem
  -> kvm_put_kvm
   -> kvm_destroy_vm
    -> kvm_destroy_devices
     -> kvm_vfio_destroy
      -> kvm_vfio_file_set_kvm
       -> vfio_file_set_kvm
        -> try to hold group->group_lock/group_rwsem

The key function is the kvm_destroy_devices() which triggers destroy cb
of kvm_device_ops. It calls back to vfio and try to hold group_lock. So
if this path doesn't call back to vfio, this dead lock would be fixed.
Actually, there is a way for it. KVM provides another point to free the
kvm-vfio device which is the point when the device file descriptor is
closed. This can be achieved by providing the release cb instead of the
destroy cb. Also rename kvm_vfio_destroy() to be kvm_vfio_release().

        /*
         * Destroy is responsible for freeing dev.
         *
         * Destroy may be called before or after destructors are called
         * on emulated I/O regions, depending on whether a reference is
         * held by a vcpu or other kvm component that gets destroyed
         * after the emulated I/O.
         */
        void (*destroy)(struct kvm_device *dev);

        /*
         * Release is an alternative method to free the device. It is
         * called when the device file descriptor is closed. Once
         * release is called, the destroy method will not be called
         * anymore as the device is removed from the device list of
         * the VM. kvm->lock is held.
         */
        void (*release)(struct kvm_device *dev);

Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM")
Reported-by: Alex Williamson <alex.william...@redhat.com>
Suggested-by: Kevin Tian <kevin.t...@intel.com>
Reviewed-by: Jason Gunthorpe <j...@nvidia.com>
Signed-off-by: Yi Liu <yi.l....@intel.com>
---
 virt/kvm/vfio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 495ceabffe88..e94f3ea718e5 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -336,7 +336,7 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
        return -ENXIO;
 }
 
-static void kvm_vfio_destroy(struct kvm_device *dev)
+static void kvm_vfio_release(struct kvm_device *dev)
 {
        struct kvm_vfio *kv = dev->private;
        struct kvm_vfio_group *kvg, *tmp;
@@ -363,7 +363,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 
type);
 static struct kvm_device_ops kvm_vfio_ops = {
        .name = "kvm-vfio",
        .create = kvm_vfio_create,
-       .destroy = kvm_vfio_destroy,
+       .release = kvm_vfio_release,
        .set_attr = kvm_vfio_set_attr,
        .has_attr = kvm_vfio_has_attr,
 };
-- 
2.34.1

Reply via email to