Skip to content

Commit

Permalink
kvm/vfio: Fix potential deadlock on vfio group_lock
Browse files Browse the repository at this point in the history
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: 421cfe6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM")
Reported-by: Alex Williamson <[email protected]>
Suggested-by: Kevin Tian <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
Reviewed-by: Matthew Rosato <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
[aw: update comment as well, s/destroy/release/]
Signed-off-by: Alex Williamson <[email protected]>
  • Loading branch information
yiliu1765 authored and awilliam committed Jan 20, 2023
1 parent 895c074 commit 51cdc8b
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions virt/kvm/vfio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -355,15 +355,15 @@ static void kvm_vfio_destroy(struct kvm_device *dev)
kvm_vfio_update_coherency(dev);

kfree(kv);
kfree(dev); /* alloc by kvm_ioctl_create_device, free by .destroy */
kfree(dev); /* alloc by kvm_ioctl_create_device, free by .release */
}

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,
};
Expand Down

0 comments on commit 51cdc8b

Please sign in to comment.