[PATCH] iommu/iommufd: Require write access for writable MAP_FILE mappings

[PATCH] iommu/iommufd: Require write access for writable MAP_FILE mappings

[Yiming Qian]

IOMMU_IOAS_MAP_FILE pins folios from a shmem/tmpfs or hugetlb file and
uses them as the backing storage for an IOAS mapping.  When userspace sets
IOMMU_IOAS_MAP_WRITEABLE, the resulting IOMMU PTEs allow DMA writes to the
file-backed folios.

The file path currently records the IOMMU mapping as writable, but it does
not require the source file descriptor to have write permission.  It also
bypasses the address_space writable-mapping accounting used by memfd
sealing.  As a result, an O_RDONLY fd for a root-owned mode 0444 shmem file
can be mapped as DMA-writeable and a device, or the IOMMUFD selftest access
path, can write into the file page cache.  The same missing accounting also
means writable IOMMU mappings are not excluded by F_SEAL_WRITE or
F_SEAL_FUTURE_WRITE.

Treat writable MAP_FILE mappings like shared writable mappings: require an
FMODE_WRITE fd, call mapping_map_writable() when creating the backing
file-pages object, and hold that accounting until the iopt_pages object is
released.  This rejects already sealed files and prevents new write seals
from being installed while the IOMMU write mapping exists.

Cc: stable@vger.kernel.org
Reported-by: Yiming Qian <yimingqian591@gmail.com>
Reported-by: Keenan Dong <keenanat2000@gmail.com>
Signed-off-by: Yiming Qian <yimingqian591@gmail.com>
Signed-off-by: Keenan Dong <keenanat2000@gmail.com>
---
 drivers/iommu/iommufd/io_pagetable.h |  1 +
 drivers/iommu/iommufd/pages.c        | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommufd/io_pagetable.h b/drivers/iommu/iommufd/io_pagetable.h
index 27e3e311d395b..63e3fd738faf2 100644
--- a/drivers/iommu/iommufd/io_pagetable.h
+++ b/drivers/iommu/iommufd/io_pagetable.h
@@ -234,6 +234,7 @@ struct iopt_pages {
 		struct {			/* IOPT_ADDRESS_FILE */
 			struct file *file;
 			unsigned long start;
+			bool mapping_writable;
 		};
 		/* IOPT_ADDRESS_DMABUF */
 		struct iopt_pages_dmabuf dmabuf;
diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
index 9bdb2945afe1e..f97d94d9eddd1 100644
--- a/drivers/iommu/iommufd/pages.c
+++ b/drivers/iommu/iommufd/pages.c
@@ -1421,13 +1421,27 @@ struct iopt_pages *iopt_alloc_file_pages(struct file *file,
 
 {
 	struct iopt_pages *pages;
+	int rc;
+
+	if (writable) {
+		if (!(file->f_mode & FMODE_WRITE))
+			return ERR_PTR(-EPERM);
+
+		rc = mapping_map_writable(file->f_mapping);
+		if (rc)
+			return ERR_PTR(rc);
+	}
 
 	pages = iopt_alloc_pages(start_byte, length, writable);
-	if (IS_ERR(pages))
+	if (IS_ERR(pages)) {
+		if (writable)
+			mapping_unmap_writable(file->f_mapping);
 		return pages;
+	}
 	pages->file = get_file(file);
 	pages->start = start - start_byte;
 	pages->type = IOPT_ADDRESS_FILE;
+	pages->mapping_writable = writable;
 	return pages;
 }
 
@@ -1668,6 +1682,8 @@ void iopt_release_pages(struct kref *kref)
 		dma_buf_put(dmabuf);
 		WARN_ON(!list_empty(&pages->dmabuf.tracker));
 	} else if (pages->type == IOPT_ADDRESS_FILE) {
+		if (pages->mapping_writable)
+			mapping_unmap_writable(pages->file->f_mapping);
 		fput(pages->file);
 	}
 	kfree(pages);

Re: [PATCH] iommu/iommufd: Require write access for writable MAP_FILE mappings

[Jason Gunthorpe]

On Sun, Jun 07, 2026 at 08:53:18AM +0000, Yiming Qian wrote:
> IOMMU_IOAS_MAP_FILE pins folios from a shmem/tmpfs or hugetlb file and
> uses them as the backing storage for an IOAS mapping.  When userspace sets
> IOMMU_IOAS_MAP_WRITEABLE, the resulting IOMMU PTEs allow DMA writes to the
> file-backed folios.

This looks like an issue with the API design in memfd_pin_folios(),
all users would have a similar bug I think.

I don't know much about memfd but this seems like a legitimate issue.

Add those involved with gup.c and the patch adding memfd_pin_folios()

>  {
>  	struct iopt_pages *pages;
> +	int rc;
> +
> +	if (writable) {
> +		if (!(file->f_mode & FMODE_WRITE))
> +			return ERR_PTR(-EPERM);
> +
> +		rc = mapping_map_writable(file->f_mapping);
> +		if (rc)
> +			return ERR_PTR(rc);
> +	}

We probably need some kind of companion API for memfd_pin_folios(), a
start/pin/destroy kind of thing to manage this?

It should not be open coded like this.

Jason

Re: [PATCH] iommu/iommufd: Require write access for writable MAP_FILE mappings

[David Hildenbrand (Arm)]

On 6/7/26 14:09, Jason Gunthorpe wrote:
> On Sun, Jun 07, 2026 at 08:53:18AM +0000, Yiming Qian wrote:
>> IOMMU_IOAS_MAP_FILE pins folios from a shmem/tmpfs or hugetlb file and
>> uses them as the backing storage for an IOAS mapping.  When userspace sets
>> IOMMU_IOAS_MAP_WRITEABLE, the resulting IOMMU PTEs allow DMA writes to the
>> file-backed folios.
> 
> This looks like an issue with the API design in memfd_pin_folios(),
> all users would have a similar bug I think.

Agreed.

Not sure if it should be part of memfd_pin_folios() itself.

> 
> I don't know much about memfd but this seems like a legitimate issue.
> 
> Add those involved with gup.c and the patch adding memfd_pin_folios()
> 
>>  {
>>  	struct iopt_pages *pages;
>> +	int rc;
>> +
>> +	if (writable) {
>> +		if (!(file->f_mode & FMODE_WRITE))
>> +			return ERR_PTR(-EPERM);
>> +
>> +		rc = mapping_map_writable(file->f_mapping);
>> +		if (rc)
>> +			return ERR_PTR(rc);
>> +	}
> 
> We probably need some kind of companion API for memfd_pin_folios(), a
> start/pin/destroy kind of thing to manage this?
> 
> It should not be open coded like this.

The permission check is one thing that's clearly missing.

Not sure about the mapping_map_writable() handling ... it's weird to rely on
that when we are not actually mmaping.

Assume we GUP a page and then munmap, mapping_unmap_writable() would be called
while we still have a writable GUP reference. Hm.

-- 
Cheers,

David

Re: [PATCH] iommu/iommufd: Require write access for writable MAP_FILE mappings

[Jason Gunthorpe]

On Mon, Jun 08, 2026 at 03:38:00PM +0200, David Hildenbrand (Arm) wrote:
> On 6/7/26 14:09, Jason Gunthorpe wrote:
> > On Sun, Jun 07, 2026 at 08:53:18AM +0000, Yiming Qian wrote:
> >> IOMMU_IOAS_MAP_FILE pins folios from a shmem/tmpfs or hugetlb file and
> >> uses them as the backing storage for an IOAS mapping.  When userspace sets
> >> IOMMU_IOAS_MAP_WRITEABLE, the resulting IOMMU PTEs allow DMA writes to the
> >> file-backed folios.
> > 
> > This looks like an issue with the API design in memfd_pin_folios(),
> > all users would have a similar bug I think.
> 
> Agreed.
> 
> Not sure if it should be part of memfd_pin_folios() itself.

I think it should, drivers should not be open coding this.

If there is such a thing as a read-only memfd then memfd_pin_folios()
should accept the usual FOLL_WRITE and deal with it internally.

> > start/pin/destroy kind of thing to manage this?
> > 
> > It should not be open coded like this.
> 
> The permission check is one thing that's clearly missing.
> 
> Not sure about the mapping_map_writable() handling ... it's weird to rely on
> that when we are not actually mmaping.

I don't know anything about sealing, but it shouldn't something check
if it is sealed read-only ?

> Assume we GUP a page and then munmap, mapping_unmap_writable() would be called
> while we still have a writable GUP reference. Hm.

I suspect the user doing the sealing has to ensure there are no pins
to the memfd before it seals it. If it already let the unsealed memfd
out of its control then it probably cannot be reliably sealed read
only?

Jason
 

Re: [PATCH] iommu/iommufd: Require write access for writable MAP_FILE mappings

[David Hildenbrand (Arm)]

On 6/8/26 15:46, Jason Gunthorpe wrote:
> On Mon, Jun 08, 2026 at 03:38:00PM +0200, David Hildenbrand (Arm) wrote:
>> On 6/7/26 14:09, Jason Gunthorpe wrote:
>>>
>>> This looks like an issue with the API design in memfd_pin_folios(),
>>> all users would have a similar bug I think.
>>
>> Agreed.
>>
>> Not sure if it should be part of memfd_pin_folios() itself.
> 
> I think it should, drivers should not be open coding this.
> 
> If there is such a thing as a read-only memfd then memfd_pin_folios()
> should accept the usual FOLL_WRITE and deal with it internally.

I'd rather not want to use FOLL flags (FOLLOW PAGE leftovers, to be renamed to
GUP_ on some lucky day) on this interface.

But sure, we could add a new set of flags (single flag for now).

> 
>>> start/pin/destroy kind of thing to manage this?
>>>
>>> It should not be open coded like this.
>>
>> The permission check is one thing that's clearly missing.
>>
>> Not sure about the mapping_map_writable() handling ... it's weird to rely on
>> that when we are not actually mmaping.
> 
> I don't know anything about sealing, but it shouldn't something check
> if it is sealed read-only ?

Right. We could do mapping_map_writable() over the duration of the function I
guess if we care about concurrent races.

Alternative have a new helper that makes sure that mapping->i_mmap_writable is
not negative (write denied).

> 
>> Assume we GUP a page and then munmap, mapping_unmap_writable() would be called
>> while we still have a writable GUP reference. Hm.
> 
> I suspect the user doing the sealing has to ensure there are no pins
> to the memfd before it seals it. If it already let the unsealed memfd
> out of its control then it probably cannot be reliably sealed read
> only?

Yes, that's my assumption. writable pins derived pre-sealing stay valid.


-- 
Cheers,

David