On Wed, Feb 25, 2026 at 12:38 AM Pranjal Shrivastava <[email protected]> wrote:
> On Thu, Jan 29, 2026 at 09:24:56PM +0000, David Matlack wrote:
> > static bool vfio_pci_liveupdate_can_finish(struct liveupdate_file_op_args
> > *args)
> > {
> > - return args->retrieved;
> > + struct vfio_pci_core_device *vdev;
> > + struct vfio_device *device;
> > +
> > + if (!args->retrieved)
> > + return false;
> > +
> > + device = vfio_device_from_file(args->file);
> > + vdev = container_of(device, struct vfio_pci_core_device, vdev);
> > +
> > + /* Check that vdev->liveupdate_incoming_state is no longer in use. */
> > + guard(mutex)(&device->dev_set->lock);
> > + return !vdev->liveupdate_incoming_state;
>
> Since we set this to NULL in the success path of vfio_pci_core_enable()
> I'm wondering if a failure in vfio_pci_core_enable could cause a
> resource leak? Because vfio_pci_liveupdate_can_finish() returns false
> as long as that pointer is valid, a single device failure will
> perpetually block the LIVEUPDATE_SESSION_FINISH IOCTL for the entire
> session preventing the LUO from reclaiming KHO memory.
>
> Shall we also set vdev->liveupdate_incoming_state = NULL on the error
> paths of vfio_pci_core_enable() ?
LIVEUPDATE_SESSION_FINISH will also perpetually fail if userspace
never calls ioctl(VFIO_DEVICE_BIND_IOMMUFD) (which is what triggers
vfio_pci_core_enable()). Or if that ioctl fails before it gets to
vfio_pci_core_enable().
It's not a great situation to be in, but this is why can_finish()
exists as a callback. Userspace must properly and correctly restore
all of the state in the session before the session can be cleaned up.
And the kernel is not going to handle every possible edge case (some
files in a session are restored but some are not), at least not
initially. If userspace gets stuck and cannot recover a resource then
userspace will have to reboot the host to get back to a healthy state.
