linux-kernel - RE: [RFC PATCH] iommufd: Destroy vdevice on device unbind

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <BN9PR11MB527677E136550FEBBF3AAFF28C78A@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Tue, 24 Jun 2025 12:24:40 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Aneesh Kumar K.V <aneesh.kumar@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>
CC: "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Joerg Roedel
	<joro@...tes.org>, Will Deacon <will@...nel.org>, Robin Murphy
	<robin.murphy@....com>
Subject: RE: [RFC PATCH] iommufd: Destroy vdevice on device unbind

> From: Aneesh Kumar K.V <aneesh.kumar@...nel.org>
> Sent: Tuesday, June 24, 2025 6:33 PM
> Jason Gunthorpe <jgg@...pe.ca> 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;
> 
> }

@release is called when a fd is closed, while the fd cannot be closed
when there is ongoing ioctl on it. So when iommufd_fops_release()
is invoked there is no parallel user-initiated operation.