[Devel] [PATCH hci-8.1 0/2] vfio: prevent kvm reference leak on qemu-update

[Devel] [PATCH hci-8.1 0/2] vfio: prevent kvm reference leak on qemu-update

[Andrey Drobyshev]

Andrey Drobyshev (2):
  vfio: move an open device's kvm reference when its group gets a new
    kvm
  KVM: remove the VM's debugfs directory when its fd is released

 drivers/vfio/group.c | 21 +++++++++++++++++++++
 virt/kvm/kvm_main.c  |  5 +++++
 2 files changed, 26 insertions(+)

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 1/2] vfio: move an open device's kvm reference when its group gets a new kvm

[Andrey Drobyshev]

A device opened through its group takes a reference to the group's kvm
and drops it only at the last close.  QEMU's cpr-exec and cpr-transfer
keep the device open while the VM is re-created: the new QEMU just adds
the group to its new kvm-vfio device.  That updates group->kvm, but the
open device keeps pinning the old kvm until the VM is shut down.  Seen
as a second KVM instance in debugfs after a live update of a vGPU VM.

Move the reference to the new kvm.  Leave NULL alone: that is the old
kvm-vfio device going away under kvm->lock, and the new kvm, if any,
arrives on its own.

https://virtuozzo.atlassian.net/browse/VSTOR-150597

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 drivers/vfio/group.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index 5f2b2c950bbc..e78c06bb7db3 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -892,9 +892,30 @@ bool vfio_group_enforced_coherent(struct vfio_group *group)
 
 void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
 {
+	struct vfio_device *device;
+
 	spin_lock(&group->kvm_ref_lock);
 	group->kvm = kvm;
 	spin_unlock(&group->kvm_ref_lock);
+
+	/*
+	 * Open devices hold a reference to the kvm they were opened with.
+	 * Move it to the new kvm, or the old one stays pinned until the
+	 * device is closed.  Nothing to move to on NULL.
+	 */
+	if (!kvm)
+		return;
+
+	mutex_lock(&group->device_lock);
+	list_for_each_entry(device, &group->device_list, group_next) {
+		mutex_lock(&device->dev_set->lock);
+		if (device->kvm && device->kvm != kvm) {
+			vfio_device_put_kvm(device);
+			vfio_device_get_kvm_safe(device, kvm);
+		}
+		mutex_unlock(&device->dev_set->lock);
+	}
+	mutex_unlock(&group->device_lock);
 }
 
 /**
-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released

[Andrey Drobyshev]

The directory under /sys/kernel/debug/kvm is named <pid>-<fd>, but is
removed only together with the VM.  A VM can outlive its fd: VFIO keeps
a reference to it for an open passthrough device, and QEMU's cpr-exec
closes the fd and creates a new VM in the same process, often on the
same fd number.  The new VM then finds its directory taken, logs
"duplicate directory" and runs without debugfs stats.  Seen on every
other live update of a vGPU VM.

Remove the directory when the fd is released: the name belongs to the
fd, which is free to reuse from then on.  kvm_destroy_vm() keeps its
call for a VM that never got an fd.

https://virtuozzo.atlassian.net/browse/VSTOR-150597

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 virt/kvm/kvm_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8d92a649c063..5e9b8a9ba8b4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1005,11 +1005,13 @@ static void kvm_destroy_vm_debugfs(struct kvm *kvm)
 		return;
 
 	debugfs_remove_recursive(kvm->debugfs_dentry);
+	kvm->debugfs_dentry = ERR_PTR(-ENOENT);
 
 	if (kvm->debugfs_stat_data) {
 		for (i = 0; i < kvm_debugfs_num_entries; i++)
 			kfree(kvm->debugfs_stat_data[i]);
 		kfree(kvm->debugfs_stat_data);
+		kvm->debugfs_stat_data = NULL;
 	}
 }
 
@@ -1364,6 +1366,9 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
 
 	kvm_irqfd_release(kvm);
 
+	/* The directory is named after this fd, which is free to reuse now */
+	kvm_destroy_vm_debugfs(kvm);
+
 	kvm_put_kvm(kvm);
 	return 0;
 }
-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released

[Konstantin Khorenko]

I won't apply this version, please send v2 with the changes below.

On Fri, 30 Oct 2026 16:49:33 +0200, Andrey Drobyshev wrote:
> +	/* The directory is named after this fd, which is free to reuse now */
> +	kvm_destroy_vm_debugfs(kvm);

Item 4 of the cover letter reply: at fd release remove only the
directory. Keep freeing debugfs_stat_data in kvm_destroy_vm(), moved
out from under the IS_ERR(kvm->debugfs_dentry) early return.

Why: an open stats file holds a reference to the kvm, not to its stat
data. kvm_debugfs_release() reads inode->i_private, the stat data, to
find the kvm it has to put, and debugfs calls it even after the file
was removed. Before this patch the stat data lived as long as the kvm,
so that was safe. Now it is freed at fd release: open
/sys/kernel/debug/kvm/<pid>-<fd>/exits, let QEMU exit, close the file,
and the release reads freed memory and puts whatever kvm pointer it
finds there. This hits any VM shutdown that races with a stats reader,
not only a live update.

> +	kvm->debugfs_dentry = ERR_PTR(-ENOENT);

Item 5 of the cover letter reply: say in the message that the destroy
uevent no longer carries STATS_PATH, and that the stats go away for a
VM that keeps running on its vcpu fds after the VM fd is closed.

Why: kvm_destroy_vm() sends the destroy uevent, and the uevent adds
STATS_PATH only for a valid dentry. Both changes look acceptable, but
a reader of the message should know about them.

Item 6 of the cover letter reply: single space after a period, and the
Feature: line.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 0/2] vfio: prevent kvm reference leak on qemu-update

[Konstantin Khorenko]

I won't apply this version, please send v2 with the changes below.

On Fri, 30 Oct 2026 16:49:31 +0200, Andrey Drobyshev wrote:
> Andrey Drobyshev (2):
>   vfio: move an open device's kvm reference when its group gets a new
>     kvm
>   KVM: remove the VM's debugfs directory when its fd is released

What to change for v2:

1. Patch 1, code and commit message: do not change device->kvm under a
   driver that still uses it after open_device(). In tree that is
   KVMGT: it registers its page-track notifier on device->kvm at open
   and unregisters it from device->kvm at close. Check what the NVIDIA
   vGPU driver does with vdev->kvm after open and say in the message
   which driver the change was tested with.

2. Patch 1, code comment and commit message: give the real reason to
   skip NULL, the lock order (details in the patch reply), and drop
   "the new kvm, if any, arrives on its own". Update the kernel-doc of
   vfio_file_set_kvm() too.

3. Patch 1, commit message: add a Fixes tag for 2b48f52f2bff ("vfio:
   fix deadlock between group lock and kvm lock").

4. Patch 2, code: at fd release remove only the directory. Keep freeing
   debugfs_stat_data in kvm_destroy_vm(), moved out from under the
   IS_ERR(kvm->debugfs_dentry) early return. This version has a
   use-after-free.

5. Patch 2, commit message: say that the destroy uevent no longer
   carries STATS_PATH, and that the stats go away for a VM that keeps
   running on its vcpu fds after the VM fd is closed.

6. Both patches: single space after a period, in the messages and in
   the new code comments, and add the Feature: line.

7. Cover letter: write one. Say what goes wrong on a live update of a
   vGPU VM, what each patch does, and how it was tested. In the subject
   "qemu-update" becomes "live update", and "kvm reference leak" covers
   patch 1 only.

Separate, not needed for v2:

1. Patch 1 should go to mainline too, I created a task for that:
   https://virtuozzo.atlassian.net/browse/VSTOR-153437

Details on items 1-6 are in the replies to the patches.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 1/2] vfio: move an open device's kvm reference when its group gets a new kvm

[Konstantin Khorenko]

I won't apply this version, please send v2 with the changes below.

On Fri, 30 Oct 2026 16:49:32 +0200, Andrey Drobyshev wrote:
> +	mutex_lock(&group->device_lock);
> +	list_for_each_entry(device, &group->device_list, group_next) {
> +		mutex_lock(&device->dev_set->lock);
> +		if (device->kvm && device->kvm != kvm) {
> +			vfio_device_put_kvm(device);
> +			vfio_device_get_kvm_safe(device, kvm);
> +		}

Item 1 of the cover letter reply: do not change device->kvm under a
driver that still uses it after open_device(). Check the NVIDIA vGPU
driver and say in the message which driver this was tested with.

Why: until now device->kvm did not change between open_device() and
close_device(), and drivers rely on that. KVMGT registers its
page-track notifier on device->kvm at open, and that takes a reference
on the kvm. At close it unregisters from device->kvm, which drops a
reference. After the move the close drops a reference on the new VM
that it never took, so the new VM is freed while QEMU still uses it.
KVMGT is not built in vz10, but the vGPU driver from the ticket is out
of tree and may do the same.

> +	/*
> +	 * Open devices hold a reference to the kvm they were opened with.
> +	 * Move it to the new kvm, or the old one stays pinned until the
> +	 * device is closed.  Nothing to move to on NULL.
> +	 */
> +	if (!kvm)
> +		return;

Item 2 of the cover letter reply: say in this comment why NULL is
skipped: the lock order. Update the kernel-doc of vfio_file_set_kvm()
too, it says the device gets the kvm when it is first opened.

Why: the NULL call from kvm_device_release() comes with kvm->lock held.
vfio_ap takes kvm->lock in its open_device(), which runs under
dev_set->lock. Taking dev_set->lock here for NULL would close an ABBA
deadlock of the kind 2b48f52f2bff fixed. "Nothing to move to" is not the
reason.

> Move the reference to the new kvm.  Leave NULL alone: that is the old
> kvm-vfio device going away under kvm->lock, and the new kvm, if any,
> arrives on its own.

Item 2 of the cover letter reply: drop "and the new kvm, if any,
arrives on its own" and give the lock order as the reason instead.

Why: with cpr-transfer the new QEMU adds the group to its kvm-vfio
first, and the old QEMU's NULL comes last. It leaves group->kvm NULL.
The open devices keep the new kvm, so the code is fine, the sentence
is not. A NULL from KVM_DEV_VFIO_FILE_DEL also holds only kv->lock,
not kvm->lock.

> https://virtuozzo.atlassian.net/browse/VSTOR-150597

Item 3 of the cover letter reply: add a Fixes tag for 2b48f52f2bff
("vfio: fix deadlock between group lock and kvm lock").

Why: that commit made the device hold a reference to the kvm until its
last close.

Item 6 of the cover letter reply: single space after a period, here
and in the code comment, and the Feature: line.

This needs to go to mainline too, I created a task for that:
https://virtuozzo.atlassian.net/browse/VSTOR-153437

Mainline vfio_group_set_kvm() also swaps only group->kvm, and since
5c3ea6d3e836d the open device even holds the old VM's file. The change
of the device->kvm contract is something the vfio maintainers have to
agree to.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel