OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
* [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update
@ 2026-11-05 12:17 Andrey Drobyshev
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm Andrey Drobyshev
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrey Drobyshev @ 2026-11-05 12:17 UTC (permalink / raw)
  To: devel

A live update keeps the passthrough device open while the VM is
re-created.  The device holds a reference to the kvm it was opened
with, so the first VM is never destroyed until the device closes, and
the new VM, on the same pid and fd number, finds its debugfs directory
taken.

Patch 1 takes that reference only for the drivers that read
device->kvm.  Patch 2 removes the debugfs directory when the VM's fd
is released.

Tested with 30 live updates of a vGPU VM (RTX 6000 Ada SR-IOV VF)
under guest DMA load, then 4 live migrations between two such nodes:
one KVM instance after every update, no "duplicate directory", no
mismatches.

v1 -> v2:

- Patch 1: the reference is no longer moved to the new kvm, which
  changed device->kvm under KVMGT; it's taken only for drivers that
  opt in with needs_kvm_ref.  Fixes tag, tested driver named

- Patch 2: only the directory is removed at fd release, the stat data
  stays until kvm_destroy_vm() (use-after-free in v1); the message
  notes the uevent and vcpu-fd changes

- Both: Feature: lines, single spaces

v1: http://lore.virtuozzo.com/20261030144933.944360-1-andrey.drobyshev@virtuozzo.com


Andrey Drobyshev (2):
  vfio: take the kvm reference only for drivers that read device->kvm
  KVM: remove the VM's debugfs directory when its fd is released

 drivers/gpu/drm/i915/gvt/kvmgt.c  |  1 +
 drivers/s390/crypto/vfio_ap_ops.c |  1 +
 drivers/vfio/pci/vfio_pci_core.c  |  1 +
 drivers/vfio/vfio_main.c          |  6 ++++--
 include/linux/vfio.h              |  6 ++++++
 virt/kvm/kvm_main.c               | 18 ++++++++++++++----
 6 files changed, 27 insertions(+), 6 deletions(-)

-- 
2.47.1

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm
  2026-11-05 12:17 [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Andrey Drobyshev
@ 2026-11-05 12:17 ` Andrey Drobyshev
  2026-11-05 12:23   ` no-reply
  2026-11-05 12:54   ` Konstantin Khorenko
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
  2026-11-05 12:54 ` [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Konstantin Khorenko
  2 siblings, 2 replies; 10+ messages in thread
From: Andrey Drobyshev @ 2026-11-05 12:17 UTC (permalink / raw)
  To: devel

A device takes a reference to its group's kvm at the first open and
drops it at the last close, so that device->kvm stays valid for the
driver in between. QEMU's cpr-exec and cpr-transfer keep the device
open while the VM is re-created, so the first VM stays referenced for
as long as the device is: its vcpus, mm and in-kernel PIT thread with
it. Seen as a second KVM instance in debugfs after a live update of a
vGPU VM.

Only three drivers read device->kvm after open_device(): KVMGT,
vfio_ap and vfio-pci's zdev support. Let them say so with
needs_kvm_ref, and take no reference for the others, which then can't
keep a VM alive. Tested with nvidia-vgpu-vfio 595.71.03 on RTX 6000
Ada SR-IOV VFs, a vfio-pci variant driver that doesn't read the
pointer.

Fixes: 2b48f52f2bff ("vfio: fix deadlock between group lock and kvm lock")
https://virtuozzo.atlassian.net/browse/VSTOR-150597
Feature: fix ms/vfio

Assisted-by: LLM
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 1 +
 drivers/s390/crypto/vfio_ap_ops.c | 1 +
 drivers/vfio/pci/vfio_pci_core.c  | 1 +
 drivers/vfio/vfio_main.c          | 6 ++++--
 include/linux/vfio.h              | 6 ++++++
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 96d23717684f..a0d2f9f4b720 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1418,6 +1418,7 @@ static int intel_vgpu_init_dev(struct vfio_device *vfio_dev)
 	int ret;
 
 	vgpu->gvt = kdev_to_i915(mdev->type->parent->dev)->gvt;
+	vfio_dev->needs_kvm_ref = true;
 	ret = intel_gvt_create_vgpu(vgpu, type->conf);
 	if (ret)
 		return ret;
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 766557547f83..78e8566a9ced 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -779,6 +779,7 @@ static int vfio_ap_mdev_init_dev(struct vfio_device *vdev)
 		container_of(vdev, struct ap_matrix_mdev, vdev);
 
 	matrix_mdev->mdev = to_mdev_device(vdev->dev);
+	vdev->needs_kvm_ref = true;
 	vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->matrix);
 	matrix_mdev->pqap_hook = handle_pqap;
 	vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->shadow_apcb);
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 7e360f6f89eb..1b02c2fb6c20 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -2094,6 +2094,7 @@ int vfio_pci_core_init_dev(struct vfio_device *core_vdev)
 
 	vdev->pdev = to_pci_dev(core_vdev->dev);
 	vdev->irq_type = VFIO_PCI_NUM_IRQS;
+	core_vdev->needs_kvm_ref = IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM);
 	mutex_init(&vdev->igate);
 	spin_lock_init(&vdev->irqlock);
 	mutex_init(&vdev->ioeventfds_lock);
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index bf01fc64ee64..dfeee692c2a4 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -441,7 +441,8 @@ void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
 
 	lockdep_assert_held(&device->dev_set->lock);
 
-	if (!kvm)
+	/* Only a driver that reads device->kvm needs the KVM referenced */
+	if (!kvm || !device->needs_kvm_ref)
 		return;
 
 	pfn = symbol_get(kvm_put_kvm);
@@ -1505,7 +1506,8 @@ static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
  * @kvm: KVM to link
  *
  * When a VFIO device is first opened the KVM will be available in
- * device->kvm if one was associated with the file.
+ * device->kvm if one was associated with the file and the driver set
+ * needs_kvm_ref.
  */
 void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
 {
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index e90859956514..96a09843e0a6 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -52,6 +52,12 @@ struct vfio_device {
 	struct vfio_device_set *dev_set;
 	struct list_head dev_set_list;
 	unsigned int migration_flags;
+	/*
+	 * Set before registration by a driver that reads device->kvm after
+	 * open_device(): the core then keeps the KVM the device was opened
+	 * with referenced until close_device().
+	 */
+	bool needs_kvm_ref;
 	struct kvm *kvm;
 
 	/* Members below here are private, not for driver use */
-- 
2.47.1

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released
  2026-11-05 12:17 [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Andrey Drobyshev
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm Andrey Drobyshev
@ 2026-11-05 12:17 ` Andrey Drobyshev
  2026-11-05 12:24   ` no-reply
  2026-11-05 12:54   ` Konstantin Khorenko
  2026-11-05 12:54 ` [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Konstantin Khorenko
  2 siblings, 2 replies; 10+ messages in thread
From: Andrey Drobyshev @ 2026-11-05 12:17 UTC (permalink / raw)
  To: devel

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 only the directory when the fd is released: the name belongs to
the fd, which is free to reuse from then on. The stat data stays until
kvm_destroy_vm(): an open stat file reaches it through the inode at
release, which debugfs runs even after the file was removed.

Two things change with that: the destroy uevent no longer carries
STATS_PATH, and a VM that lives on after its fd on the vcpu fds has
no stats directory for that time.

https://virtuozzo.atlassian.net/browse/VSTOR-150597
Feature: fix ms/kvm

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8d92a649c063..9a0ee933a6a3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -995,21 +995,28 @@ static umode_t kvm_stats_debugfs_mode(const struct _kvm_stats_desc *pdesc)
 }
 
 
+static void kvm_remove_vm_debugfs(struct kvm *kvm)
+{
+	if (IS_ERR(kvm->debugfs_dentry))
+		return;
+
+	debugfs_remove_recursive(kvm->debugfs_dentry);
+	kvm->debugfs_dentry = ERR_PTR(-ENOENT);
+}
+
 static void kvm_destroy_vm_debugfs(struct kvm *kvm)
 {
 	int i;
 	int kvm_debugfs_num_entries = kvm_vm_stats_header.num_desc +
 				      kvm_vcpu_stats_header.num_desc;
 
-	if (IS_ERR(kvm->debugfs_dentry))
-		return;
-
-	debugfs_remove_recursive(kvm->debugfs_dentry);
+	kvm_remove_vm_debugfs(kvm);
 
 	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 +1371,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_remove_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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm Andrey Drobyshev
@ 2026-11-05 12:23   ` no-reply
  2026-11-05 12:54   ` Konstantin Khorenko
  1 sibling, 0 replies; 10+ messages in thread
From: no-reply @ 2026-11-05 12:23 UTC (permalink / raw)
  To: devel, Andrey Drobyshev

> A device takes a reference to its group's kvm at the first open and
> drops it at the last close, so that device->kvm stays valid for the
> driver in between. QEMU's cpr-exec and cpr-transfer keep the device
> open while the VM is re-created, so the first VM stays referenced for
> as long as the device is: its vcpus, mm and in-kernel PIT thread with
> it. Seen as a second KVM instance in debugfs after a live update of a
> vGPU VM.
> 
> Only three drivers read device->kvm after open_device(): KVMGT,
> vfio_ap and vfio-pci's zdev support. Let them say so with
> needs_kvm_ref, and take no reference for the others, which then can't
> keep a VM alive. Tested with nvidia-vgpu-vfio 595.71.03 on RTX 6000
> Ada SR-IOV VFs, a vfio-pci variant driver that doesn't read the
> pointer.
> 
> Fixes: 2b48f52f2bff ("vfio: fix deadlock between group lock and kvm lock")
> https://virtuozzo.atlassian.net/browse/VSTOR-150597
> Feature: fix ms/vfio
> 
> Assisted-by: LLM
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105121733.643754-1-andrey.drobyshev@virtuozzo.com?part=1


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
@ 2026-11-05 12:24   ` no-reply
  2026-11-05 12:54   ` Konstantin Khorenko
  1 sibling, 0 replies; 10+ messages in thread
From: no-reply @ 2026-11-05 12:24 UTC (permalink / raw)
  To: Andrey Drobyshev, devel

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] KVM: debugfs dentry removed at VM fd release while vcpu/device fd paths still use it
- [Low] kvm: destroy uevent loses STATS_PATH once VM fd is released
- [Low] KVM: debugfs dir removed at file release, not when the fd number is freed
--

commit beaae50d646de519b7a984e475c82532587d2d5a
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

KVM: remove the VM's debugfs directory when its fd is released

This patch removes the per-VM debugfs directory in kvm_vm_release(), when
the VM fd is released, instead of only in kvm_destroy_vm(). The stat data
is still freed at VM destruction. This is meant to avoid "duplicate
directory" collisions when a VM outlives its fd and the fd number is reused.

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

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c

[ ... ]

> +static void kvm_remove_vm_debugfs(struct kvm *kvm)
> +{
> +	if (IS_ERR(kvm->debugfs_dentry))
> +		return;
> +
> +	debugfs_remove_recursive(kvm->debugfs_dentry);
> +	kvm->debugfs_dentry = ERR_PTR(-ENOENT);
> +}

[Severity: High]
Can this race with readers of kvm->debugfs_dentry that hold only a kvm
reference and not the VM fd?

Before this patch the dentry was removed only in kvm_destroy_vm(), when
users_count had reached zero, so any holder of a kvm reference could use it.
Now the dentry is removed while struct kvm stays live through vcpu fds,
device fds or VFIO.

kvm_remove_vm_debugfs() removes the directory and then stores
ERR_PTR(-ENOENT) with a plain store, with no lock and no
READ_ONCE()/WRITE_ONCE(). Consider these two paths on arm64.

vcpu fd path, on first KVM_RUN:

vgic_lazy_init()
  vgic_init()
    vgic_debug_init(kvm)
      debugfs_create_file("vgic-state", 0444, kvm->debugfs_dentry, kvm,
                          &vgic_debug_fops);

Only kvm->arch.config_lock is held here, and kvm_vm_release() never takes
it.

device fd path:

KVM_SET_DEVICE_ATTR (KVM_VGIC_ITS_ADDR_TYPE) on an ITS device fd
  vgic_its_debug_init()
    debugfs_create_file(name, 0444, dev->kvm->debugfs_dentry, its,
                        &vgic_its_debug_fops);

A device fd pins struct kvm but not the VM fd. The same applies to
KVM_DEV_ARM_VGIC_GRP_CTRL reaching vgic_init() and then vgic_debug_init().

If another thread closes the last VM fd, a reader that loaded the old
pointer before the store reaches start_creating() in fs/debugfs/inode.c:

start_creating()
    if (IS_ERR(parent))
        return parent;
    ...
    inode_lock(d_inode(parent));
    if (unlikely(IS_DEADDIR(d_inode(parent))))

The IS_ERR(parent) check only helps a reader that sees the new value.
Otherwise start_creating() locks and inspects a dentry and inode that may
already be freed, or creates a file under a directory that is being removed.

Can this cause a use-after-free of the debugfs parent dentry and inode? The
window is narrow, but it matches the cpr-exec/VFIO scenario this patch
targets.

Should these users be serialized against kvm_vm_release()?

[ ... ]

> @@ -1364,6 +1371,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_remove_vm_debugfs(kvm);
> +

[Severity: Low]
Is the comment accurate? kvm_vm_release() runs at the last fput() of the
struct file, not when the fd number is freed.

close_fd() frees the fd slot under file_lock first:

close_fd()
    spin_lock(&files->file_lock);
    file = file_close_fd_locked(files, fd);
    spin_unlock(&files->file_lock);
    ...
    return filp_close(file, files);

The final fput() then runs later as task work. In a multithreaded process,
another thread can get the same fd number and call KVM_CREATE_VM before the
deferred release has run.

If the file has other holders (dup(), SCM_RIGHTS, another process), the fd
number is reusable while the old directory still exists.

In both cases kvm_create_vm_debugfs() still hits this path:

snprintf(dir_name, sizeof(dir_name), "%d-%s", task_pid_nr(current), fdname);
mutex_lock(&kvm_debugfs_lock);
dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
if (dent) {
    pr_warn_ratelimited("KVM: debugfs: duplicate directory %s\n", dir_name);

The new VM then runs without debugfs stats, as in the original problem. Does
the claim that the name is free to reuse once the fd is released hold only
for the simple single-threaded close-then-create case?

>  	kvm_put_kvm(kvm);
>  	return 0;
>  }

[Severity: Low]
This isn't a crash, but the destroy uevent no longer carries STATS_PATH in
the usual case. kvm_remove_vm_debugfs() sets kvm->debugfs_dentry to
ERR_PTR(-ENOENT) before the final kvm_put_kvm().

The final kvm_put_kvm() then reaches kvm_destroy_vm(), which calls:

kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
kvm_destroy_vm_debugfs(kvm);

In kvm_uevent_notify_change() the STATS_PATH block is guarded by
if (!IS_ERR(kvm->debugfs_dentry)), so it is skipped.

This is not limited to the VFIO and cpr-exec cases. Whenever the VM fd is
closed before the last kvm reference is dropped, the destroy uevent omits
STATS_PATH. Before the patch it was always present. The commit message
mentions this change, but is the userspace-visible change to the destroy
uevent intended for all VMs?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105121733.643754-1-andrey.drobyshev@virtuozzo.com?part=2

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update
  2026-11-05 12:17 [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Andrey Drobyshev
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm Andrey Drobyshev
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
@ 2026-11-05 12:54 ` Konstantin Khorenko
  2 siblings, 0 replies; 10+ messages in thread
From: Konstantin Khorenko @ 2026-11-05 12:54 UTC (permalink / raw)
  To: Andrey Drobyshev; +Cc: devel

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

On Thu, 5 Nov 2026 14:17:31 +0200, Andrey Drobyshev wrote:
> Patch 1 takes that reference only for the drivers that read
> device->kvm.  Patch 2 removes the debugfs directory when the VM's fd
> is released.

Patch 1 is right in the code now, thanks. What to change for v3:

1. Patch 1, code comment and commit message: the three drivers read
   device->kvm in open_device(), not after it, and vfio_ap reads it
   only there. Say also that device->kvm stays NULL for a driver
   without the flag, not only that no reference is taken.

2. Patch 1, commit message: say how you checked that nvidia-vgpu-vfio
   does not read device->kvm.

3. Patch 2: test patch 1 alone. With it, nothing should keep the old
   VM alive across cpr-exec on your setup, so the duplicate directory
   should be gone too. If it is, drop patch 2. If it is not, the
   message has to name what still holds the VM, since "VFIO keeps a
   reference" is no longer true for your driver.

4. Patch 2, code, if it stays: rename the directory with
   debugfs_change_name() to a name that cannot clash, instead of
   removing it. Removing it opens a use-after-free on powerpc. A
   rename also keeps STATS_PATH and the vcpu stats, so the "Two things
   change" paragraph goes away.

5. Both patches: drop "Assisted-by: LLM" from the commits, since
   2026-10-22 it goes only into the cover letter, as one line before
   the sign-off.

6. Cover letter: single space after a period (three places), the
   Assisted-by line, and the result of item 3. Say what "no
   mismatches" compares, or drop it.

Details 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm Andrey Drobyshev
  2026-11-05 12:23   ` no-reply
@ 2026-11-05 12:54   ` Konstantin Khorenko
  1 sibling, 0 replies; 10+ messages in thread
From: Konstantin Khorenko @ 2026-11-05 12:54 UTC (permalink / raw)
  To: Andrey Drobyshev; +Cc: devel

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

On Thu, 5 Nov 2026 14:17:32 +0200, Andrey Drobyshev wrote:
> Only three drivers read device->kvm after open_device(): KVMGT,
> vfio_ap and vfio-pci's zdev support. Let them say so with
> needs_kvm_ref, and take no reference for the others, which then can't
> keep a VM alive. Tested with nvidia-vgpu-vfio 595.71.03 on RTX 6000
> Ada SR-IOV VFs, a vfio-pci variant driver that doesn't read the
> pointer.

Item 1 of the cover letter reply: "after open_device()" becomes "in
open_device()", and say that device->kvm stays NULL without the flag.

Why: all three read the pointer in open_device(). vfio_ap reads it
only there and fails the open with -EINVAL when it is NULL. And
vfio_device_get_kvm_safe() returns before it sets device->kvm, so a
driver without the flag gets no pointer at all, not just no
reference. An out-of-tree driver that reads it now sees NULL.

Item 2 of the cover letter reply: say how you checked that
nvidia-vgpu-vfio does not read device->kvm.

Why: that driver is the one this change is for, and its source is not
in our tree. If it does read the pointer, it now gets NULL.

> +	/*
> +	 * Set before registration by a driver that reads device->kvm after
> +	 * open_device(): the core then keeps the KVM the device was opened
> +	 * with referenced until close_device().
> +	 */

Item 1 of the cover letter reply: the same two fixes in this comment.

> Feature: fix ms/vfio
>
> Assisted-by: LLM

Item 5 of the cover letter reply: drop the Assisted-by line here, it
goes into the cover letter. Put a blank line before Feature:.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released
  2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
  2026-11-05 12:24   ` no-reply
@ 2026-11-05 12:54   ` Konstantin Khorenko
  1 sibling, 0 replies; 10+ messages in thread
From: Konstantin Khorenko @ 2026-11-05 12:54 UTC (permalink / raw)
  To: Andrey Drobyshev; +Cc: devel

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

On Thu, 5 Nov 2026 14:17:33 +0200, Andrey Drobyshev wrote:
> 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.

Item 3 of the cover letter reply: test patch 1 alone. If the duplicate
directory is gone, drop this patch. If not, name in the message what
still holds the old VM.

Why: with patch 1, VFIO takes no kvm reference for nvidia-vgpu-vfio.
The VM fd, the vcpu fds and the device fds are all close-on-exec. So
on cpr-exec the old VM is destroyed, and its directory removed, before
the new QEMU creates its VM. What can still hold a VM past its fd is
KVMGT, vfio_ap, zdev, a guest_memfd fd or an open stats file, and
none of the three drivers is built in our x86_64 kernels.

> +	/* The directory is named after this fd, which is free to reuse now */
> +	kvm_remove_vm_debugfs(kvm);

Item 4 of the cover letter reply: if the patch stays, rename the
directory with debugfs_change_name() to a name that cannot clash,
instead of removing it. Then drop the "Two things change" paragraph.

Why: on powerpc the XICS and XIVE devices create a file in this
directory and remove it when their device fd is released. If the VM fd
is closed first, the recursive removal here frees that dentry, and the
device release then removes freed memory. Our x86_64 kernels are not
hit, but the code should not carry that. My v1 reply asked to remove
only the directory and missed this case, sorry. A rename keeps every
child dentry valid until kvm_destroy_vm(), and STATS_PATH and the vcpu
stats keep working.

> Feature: fix ms/kvm
>
> Assisted-by: LLM

Item 5 of the cover letter reply: drop the Assisted-by line here, it
goes into the cover letter. Put a blank line before Feature:.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released
  2026-10-30 14:49 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
@ 2026-11-04 20:08   ` Konstantin Khorenko
  0 siblings, 0 replies; 10+ messages in thread
From: Konstantin Khorenko @ 2026-11-04 20:08 UTC (permalink / raw)
  To: Andrey Drobyshev; +Cc: devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released
  2026-10-30 14:49 [Devel] [PATCH hci-8.1 0/2] vfio: prevent kvm reference leak on qemu-update Andrey Drobyshev
@ 2026-10-30 14:49 ` Andrey Drobyshev
  2026-11-04 20:08   ` Konstantin Khorenko
  0 siblings, 1 reply; 10+ messages in thread
From: Andrey Drobyshev @ 2026-10-30 14:49 UTC (permalink / raw)
  To: devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-11-05 12:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-05 12:17 [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Andrey Drobyshev
2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 1/2] vfio: take the kvm reference only for drivers that read device->kvm Andrey Drobyshev
2026-11-05 12:23   ` no-reply
2026-11-05 12:54   ` Konstantin Khorenko
2026-11-05 12:17 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
2026-11-05 12:24   ` no-reply
2026-11-05 12:54   ` Konstantin Khorenko
2026-11-05 12:54 ` [Devel] [PATCH hci-8.1 v2 0/2] vfio, KVM: kvm reference leak and debugfs clash on live update Konstantin Khorenko
  -- strict thread matches above, loose matches on Subject: below --
2026-10-30 14:49 [Devel] [PATCH hci-8.1 0/2] vfio: prevent kvm reference leak on qemu-update Andrey Drobyshev
2026-10-30 14:49 ` [Devel] [PATCH hci-8.1 2/2] KVM: remove the VM's debugfs directory when its fd is released Andrey Drobyshev
2026-11-04 20:08   ` Konstantin Khorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox