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] [thread-next>] [day] [month] [year] [list]

Message-ID: <yq5att4drtj7.fsf@kernel.org>
Date: Wed, 18 Jun 2025 20:22:44 +0530
From: Aneesh Kumar K.V <aneesh.kumar@...nel.org>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: "Tian, Kevin" <kevin.tian@...el.com>,
	"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

Jason Gunthorpe <jgg@...pe.ca> writes:

> On Wed, Jun 18, 2025 at 10:59:00AM +0530, Aneesh Kumar K.V wrote:
>> Jason Gunthorpe <jgg@...pe.ca> writes:
>> 
>> > On Tue, Jun 17, 2025 at 01:37:04PM +0530, Aneesh Kumar K.V wrote:
>> >  
>> >> How do we reclaim that object id for further reuse? 
>> >
>> > Maybe just don't? Userspace did something it shouldn't, it now leaked
>> > 8 bytes of kernel memory until the FD is closed.
>> >
>> 
>> Between the two sequences below, Sequence 1 is the correct one, since we
>> want the object ID to be released after calling ioctl(DESTROY,
>> vdevice_id), right?
>> 
>> Sequence 1 (Correct):
>> 
>> close(vfio_cdev)          → triggers vdevice destruction  
>> ioctl(DESTROY, vdevice_id) → reclaims vdevice object ID  
>> close(iommufd)  
>
> This is wrong, the vdevice has outlived the idevice
>
>> Sequence 2:
>> 
>> ioctl(DESTROY, vdevice_id) → returns EBUSY
>
> It should not return EBUSY, it should destry the vdevice.
>
> 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).
ioctl(iommufd, IOMMUFD_CMD_VDEVICE_DEALLOC) -> No error, we convert the XA_ZERO_ENTRY to NULL here?
ioctl(iommufd, IOMMUFD_CMD_VIOMMU_DEALLOC)

-aneesh