Re: [PATCH v7 07/16] iommufd/device: Associate KVM file pointer with iommufd_device

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Oct 12, 2026 at 02:03:13PM +0530, Aneesh Kumar K.V wrote:
> sashiko-bot@kernel.org writes:
> 
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] iommufd: UAF of kvm_file due to iommufd_device outliving VFIO unbind
> > --
> >
> > commit ddde31c06624779bad69fb77ae7b9503b553dacc
> > Author: Shameer Kolothum <skolothumtho@nvidia.com>
> >
> > iommufd/device: Associate KVM file pointer with iommufd_device
> >
> > TSM vDevice support needs access to the KVM associated with a VFIO device
> > after the device has been bound to iommufd. Extend iommufd_device_bind() to
> > accept the device's KVM file and store it in the iommufd_device.
> >
> >> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> >> index a664c70a6fe73..f99f7bee2e774 100644
> >> --- a/drivers/iommu/iommufd/device.c
> >> +++ b/drivers/iommu/iommufd/device.c
> > [ ... ]
> >> @@ -308,7 +309,9 @@ static int iommufd_bind_noiommu(struct iommufd_device *idev)
> >>   * The caller must undo this with iommufd_device_unbind()
> >>   */
> >>  struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> >> -					   struct device *dev, u32 *id)
> >> +					   struct device *dev,
> >> +					   struct file *kvm_file,
> >> +					   u32 *id)
> >>  {
> >>  	struct iommufd_device *idev;
> >>  	int rc;
> >> @@ -319,6 +322,8 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> >>  
> >>  	idev->ictx = ictx;
> >>  	idev->dev = dev;
> >> +	/* VFIO holds the file reference while the device is open. */
> >> +	idev->kvm_file = kvm_file;
> >
> > [Severity: High]
> > Does this code introduce a use-after-free regression for kvm_file when a
> > concurrent ioctl is accessing the device?
> >
> > If an untrusted user thread executes an ioctl that calls iommufd_get_device(),
> > it will increment idev->obj.users to 2.
> >
> > If another thread concurrently closes the VFIO device fd, it calls
> > vfio_df_unbind_iommufd() which leads to iommufd_device_unbind() and then
> > iommufd_object_destroy_user().
> >
> > Because users is 2, iommufd_object_remove() will return -EBUSY:
> >
> > drivers/iommu/iommufd/main.c:iommufd_object_remove() {
> >         ...
> >         if (!refcount_dec_if_one(&obj->users)) {
> >                 ret = -EBUSY;
> >                 goto err_xa;
> >         }
> >         ...
> > }
> >
> > The unbind operation ignores this error, leaving the idev alive:
> >
> > drivers/iommu/iommufd/iommufd_private.h:iommufd_object_destroy_user() {
> >         ...
> >         ret = iommufd_object_remove(ictx, obj, obj->id, REMOVE_WAIT);
> >         WARN_ON(ret);

REMOVE_WAIT is not supposed to fail, that is why there is a
WARN_ON. If we hit a bug which causes this you get a warn on and other
stuff like UAFs too.

Don't worry about it.

Jason