This is a note to let you know that I've just added the patch titled kvm/vfio: Fix potential deadlock on vfio group_lock to the 6.1-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: kvm-vfio-fix-potential-deadlock-on-vfio-group_lock.patch and it can be found in the queue-6.1 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. commit fd33207736a2552bd0d5f6c62a4f115b1e09da5f Author: Yi Liu <yi.l.liu@xxxxxxxxx> Date: Fri Jan 20 07:05:28 2023 -0800 kvm/vfio: Fix potential deadlock on vfio group_lock [ Upstream commit 51cdc8bc120ef6e42f6fb758341f5d91bc955952 ] 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.williamson@xxxxxxxxxx> Suggested-by: Kevin Tian <kevin.tian@xxxxxxxxx> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx> Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx> Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx> Link: https://lore.kernel.org/r/20230114000351.115444-1-mjrosato@xxxxxxxxxxxxx Link: https://lore.kernel.org/r/20230120150528.471752-1-yi.l.liu@xxxxxxxxx [aw: update comment as well, s/destroy/release/] Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c index 495ceabffe88..9584eb57e0ed 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; @@ -355,7 +355,7 @@ 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); @@ -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, };