Re: [RFC PATCH] iommufd: Destroy vdevice on device unbind

[Aneesh Kumar K . V]

Jason Gunthorpe <jgg@xxxxxxxx> writes:

> On Wed, Jun 18, 2025 at 08:22:44PM +0530, Aneesh Kumar K.V wrote:
>> > The full sequence I would expect a sane userspace to do is:
>> >
>> > open(vfio_cdev)
>> > ioctl(vfio_cdev, VFIO_DEVICE_BIND_IOMMUFD, iommufd)
>> > ioctl(iommufd, IOMMUFD_CMD_VIOMMU_ALLOC)
>> > ioctl(iommufd, IOMMUFD_CMD_VDEVICE_ALLOC)
>> > ioctl(iommufd, IOMMUFD_CMD_VDEVICE_DEALLOC)
>> > ioctl(iommufd, IOMMUFD_CMD_VIOMMU_DEALLOC)
>> > close(vfio_cdev);
>> >
>> 
>> And if the user does
>> 
>> open(vfio_cdev)
>> ioctl(vfio_cdev, VFIO_DEVICE_BIND_IOMMUFD, iommufd)
>> ioctl(iommufd, IOMMUFD_CMD_VIOMMU_ALLOC)
>> ioctl(iommufd, IOMMUFD_CMD_VDEVICE_ALLOC)
>> close(vfio_cdev);   -> this should call vdevice_destroy because idevice is getting destroyed here (we will put XA_ZERO_ENTRY here).
>
> Yes, we have to destroy the vdevice internally here
>
>> ioctl(iommufd, IOMMUFD_CMD_VDEVICE_DEALLOC) -> No error, we convert the XA_ZERO_ENTRY to NULL here?
>
> This should probably fail since the user has done something wrong and
> it would be the only way to realize it. The failure could clean up the
> tombstone, or it could just leak I don't have a strong feeling.
>
> If you leak then using XA_ZERO_ENTRY is easy, if you want to clean up
> then you'd have to have a global static 'tombstone object' that sits
> in the xarray.

I have a related question w.r.t iommufd_fops_release(). How is that safe
against a parallel iommufd_destroy()?

in iommufd_fops_release ()
xa_for_each(&ictx->objects, index, obj) {

                            ---> A parallel iommufd_destroy() can free the obj here ?

	if (!refcount_dec_if_one(&obj->users))
		continue;

}

-aneesh