Re: [PATCH v3 4/5] KVM/vfio: Use file-based reference counting for KVM

[Christian Borntraeger <borntraeger@linux.ibm.com>]

Am 24.09.26 um 11:14 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] kvm/vfio: memory leak due to circular file reference

So with the help of AI drawing things I think this is a false positive, see below.


References between KVM and VFIO after
"KVM/vfio: Use file-based reference counting for KVM" v3

   ====>  ||  \/  /\   holds a reference (get_file(), kvm_get_kvm(), fd)
   ---->  :   v        plain pointer, holds no reference

                process fd table (exit_files() drops every fd)
         ||               ||                 ||               ||
         \/               \/                 \/               \/
  +--------------+  +--------------+  +---------------+  +-----------+
  | kvm-vfio     |  | VFIO group   |  | VFIO device   |  | VM file   |
  | device file  |  | or cdev file |  | file (opened) |  | "kvm-vm"  |
  +--------------+  +--------------+  +---------------+  +-----------+
    ||   ||            /\     ||              ||            /\   ||
    ||   || kvf->file  ||     || group->kvm   || device->kvm||   || private_data
    ||   ++============++     || df->kvm      ++============++   || users_count
    ||                        || (NEW)                      ||   ||
    || dev->kvm               ++============================++   ||
    || users_count                                               \/
    ||                                                   +-------------+
    ++==================================================>| struct kvm  |
                                                         +-------------+
                                                           :         :
                                              kvm->devices :         : kvm->file
                                                (no ref)   :         : (no ref)
                                              to kvm-vfio  v         v to the
                                                device                 VM file


Every ====> arrow points to the right or down.  Nothing leaves the
VM file or struct kvm with a ====> back to a VFIO file or to the
kvm-vfio device file; the two arrows that do point back are plain
pointers.  The kvm-vfio device file is held only by the fd table,
so it is always released first, and its .release takes the
kvf->file and group->kvm/df->kvm arrows away:






[...]

> > -void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
> > +void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm)
> >  {
> > +   struct file *old;
> > +
> > +   if (kvm)
> > +           get_file(kvm);
> > +
> >     spin_lock(&group->kvm_ref_lock);
> > +   old = group->kvm;
> >     group->kvm = kvm;
> >     spin_unlock(&group->kvm_ref_lock);
>
> [Severity: High]
> Does this create a circular file reference? VFIO takes a reference on the KVM VM
> file here in vfio_group_set_kvm(), while KVM also takes a reference on the
> VFIO file.

And this (also for the other aspects) might be the relevant part.
The VM file does not hold the reference on the VFIO file.  The
reference is held by the kvm-vfio device (kvf->file in struct kvm_vfio),
and that device is owned by its own fd, not by the VM file or by
struct kvm.