RE: [PATCH v8 06/15] iommufd/device: Hold a KVM file reference for the device lifetime

[Tian, Kevin]

> From: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
> Sent: Thursday, November 5, 2026 12:16 AM
> 
> The KVM file stored in an iommufd_device currently relies on the
> reference held by VFIO while the device is open. Normally, unbind waits
> for outstanding device references before VFIO drops its KVM reference.
> However, REMOVE_WAIT only waits for 60 seconds. If that timeout expires,
> the device object remains in the iommufd xarray.
> 
> For example, an IOMMU_GET_HW_INFO ioctl can hold a device reference
> while
> blocked on a userspace buffer fault:
> 
>   iommufd_fops_ioctl()
>     iommufd_get_hw_info()
>       iommufd_get_device()
>         iommufd_get_object()
>           iommufd_lock_obj()       /* takes users and wait_cnt refs */
>       copy_to_user() / clear_user() /* may block before putting refs */
> 
> A concurrent final release of the VFIO device file takes this path:
> 
>   vfio_device_fops_release()
>     vfio_df_unbind_iommufd()
>       vfio_df_close()
>         vfio_df_device_last_close()
>           vfio_df_iommufd_unbind()
>             vfio_iommufd_physical_unbind()
>               iommufd_device_unbind()
>                 iommufd_object_destroy_user()
>                   iommufd_object_remove(..., REMOVE_WAIT)
>                     iommufd_object_dec_wait() /* timeout: -EBUSY */
>                   WARN_ON(ret)
>       vfio_device_put_kvm()
>         fput(device->kvm)
> 
> iommufd_object_destroy_user() warns on -EBUSY but does not propagate it:
> iommufd_device_unbind() returns void. Thus VFIO close effectively ignores
> the unbind failure and proceeds to drop its KVM file reference even
> though the iommufd_device is still alive and its ID remains usable.
> 
> Once the remaining KVM file references are released, a later ioctl on
> the still-open iommufd can access the stale pointer:
> 
>   iommufd_fops_ioctl()
>     iommufd_viommu_alloc_ioctl()
>       iommufd_get_device()         /* finds the surviving device ID */
>       get_file(idev->kvm_file)     /* accesses the freed KVM file */
> 
> Take a KVM file reference when binding the device and release it when
> the iommufd_device is actually destroyed. Release it before the igroup
> check in the destructor so that bind failure cleanup is covered too.
> Document the ownership in the bind API and device structure.

I don't think this needs to be fixed. There is already a WARN_ON()
in iommufd_object_destroy_user() and many bad things could
happen afterwards...

If we really think an ioctl could take more than 60s in sane scenarios,
it's a more complex issue to be handled in iommufd core.

> 
> Fixes: d8bb80c8f0e5 ("iommufd/device: Associate KVM file pointer with
> iommufd_device")

btw no fix tag for an issue introduced by an earlier patch in the
same series.