[PATCH] iommufd: reject mapping an iommufd file into an IOAS

[PATCH] iommufd: reject mapping an iommufd file into an IOAS

[Jiahao Xu]

iopt_map_file_pages() takes the fd passed to IOMMU_IOAS_MAP_FILE and, for
every file that is not a dma-buf, stores a reference to it in the pages
of the new mapping:

	iopt_map_file_pages()
	  iopt_alloc_file_pages()
	    pages->file = get_file(file)

iopt_release_pages() drops that reference only when the iopt_pages is
destroyed, which for a file mapping happens while the iommufd file is
being torn down.

Nothing validates what kind of file the fd refers to, so userspace can
pass the iommufd file itself, or two iommufd files whose IOASs map each
other.  The reference graph then contains a cycle:

	struct file (/dev/iommu) -> iommufd_ctx -> iommufd_ioas
	  -> io_pagetable -> iopt_area -> iopt_pages -> get_file(file)

Closing the fd drops no reference at all: the file's refcount cannot
reach zero while the iommufd_ctx it owns still holds a reference to it,
so iommufd_fops_release() never runs and the iopt_pages, the IOAS, the
iommufd_ctx and the struct file are all leaked.  kmemleak reports the
leak at the file allocation site ("memory leak in path_openat"), while
the holder stack of the same report shows the reference being taken
through iommufd_ioas_map_file() -> iopt_map_file_pages() ->
iopt_alloc_file_pages().

Reject an iommufd file before the reference is established, as Christian
Brauner suggested when the report was discussed ("this should either
reject based on file->f_op == &iommufd_fops").  iommufd_ctx_from_file()
already performs exactly that comparison against the static iommufd_fops
and is part of the public iommufd interface, so the check needs neither a
new symbol nor a new export and the fix fits in one file.  The extra
context reference it returns is dropped again immediately.

The check must not be placed after iopt_alloc_file_pages(): on an IOAS
with no attached domain the mapping can succeed without pinning
anything, so the reference is handed over to the iopt_area and the cycle
already exists, with no sensible place left to unwind it.

Limitations: this only rejects iommufd files.  A vfio device fd that is
bound to the same iommufd also holds an iommufd_ctx reference
(iommufd_ctx_from_fd()) and mapping such an fd can still build a cycle,
as Brauner pointed out.  This is also not the general validation of
which files may be mapped (memfd/shmem/hugepage) that Jason Gunthorpe
asked for in order to keep the memfd_pin_folios() decision on the mm
side.  Both remain to be addressed separately.

Fixes: f4986a72d6e4 ("iommufd: Add IOMMU_IOAS_MAP_FILE")
Reported-by: syzbot+7666ed2c3d42196bf7e3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7666ed2c3d42196bf7e3
Link: https://lore.kernel.org/all/20260904-clown-zersplittern-sequenz-987f972a607b@brauner/
Signed-off-by: Jiahao Xu <xujiahao@cmss.chinamobile.com>
---
 drivers/iommu/iommufd/io_pagetable.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c
index 5ac736cf5..fca14b012 100644
--- a/drivers/iommu/iommufd/io_pagetable.c
+++ b/drivers/iommu/iommufd/io_pagetable.c
@@ -510,11 +510,26 @@ int iopt_map_file_pages(struct iommufd_ctx *ictx, struct io_pagetable *iopt,
 		}
 	} else {
 		struct file *file;
+		struct iommufd_ctx *file_ictx;
 
 		file = fget(fd);
 		if (!file)
 			return -EBADF;
 
+		/*
+		 * Mapping an iommufd file would create a reference cycle:
+		 * file -> iommufd_ctx -> ioas -> iopt -> iopt_pages -> file.
+		 * The get_file() below can then never be released, since the
+		 * context that owns the file can never be destroyed.  Reject it
+		 * before the reference is transferred to the pages.
+		 */
+		file_ictx = iommufd_ctx_from_file(file);
+		if (!IS_ERR(file_ictx)) {
+			iommufd_ctx_put(file_ictx);
+			fput(file);
+			return -EINVAL;
+		}
+
 		pages = iopt_alloc_file_pages(file, start_byte, start, length,
 					      iommu_prot & IOMMU_WRITE);
 		fput(file);
-- 
2.43.0

Re: [PATCH] iommufd: reject mapping an iommufd file into an IOAS

[Jason Gunthorpe]

On Thu, Nov 12, 2026 at 10:59:45AM +0800, Jiahao Xu wrote:
> iopt_map_file_pages() takes the fd passed to IOMMU_IOAS_MAP_FILE and, for
> every file that is not a dma-buf, stores a reference to it in the pages
> of the new mapping:
> 
> 	iopt_map_file_pages()
> 	  iopt_alloc_file_pages()
> 	    pages->file = get_file(file)

I've know about this for a bit but haven't got to figuring out a
solution.

This patch is not quite good enough as any fd that can hold an
iommufd, like vfio is also a problem.

Really it needs to be reversed so only a memfd or dmabuf is held, but
I don't know how to do that..

Jason