[PATCH 0/2] iommufd: remove the last use of set_page_dirty_lock()

[PATCH 0/2] iommufd: remove the last use of set_page_dirty_lock()

[Tal Zussman]

batch_rw() copies to and from pinned user pages one page at a time and
dirties each with set_page_dirty_lock(). The second patch walks the pfn
batch a folio at a time with memcpy_to_folio() and memcpy_from_folio()
and marks each folio dirty once. It removes the last use of
set_page_dirty_lock() in iommufd.

The issue fixed in the first patch was found while testing the second:
iommufd_fail_nth turned up a dev_iommu leak in iommu_fwspec_init() under
kmemleak. The fix is independent and could go through the iommu tree on
its own.

Tested on x86 under virtme-ng with KASAN and lockdep against the iommufd
mock driver. The selftests and iommufd_fail_nth all pass.

With ftrace timing iopt_pages_rw_slow() through the mock driver on a
non-debug kernel, slow-path writes to THP-backed pages drop from ~1310 to
~930 ns/page (now the same cost as a read), and writes to 4K folios drop
~9%.

---
Tal Zussman (2):
      iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec
      iommufd: copy to and from folios in batch_rw()

 drivers/iommu/iommu.c         |  6 +++++-
 drivers/iommu/iommufd/pages.c | 35 ++++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 16 deletions(-)
---
base-commit: 04bcf84613346b973a8cc0c59238b6b5bc902688
change-id: 20261015-iommufd-folio-dirty-8abff4281f3a

Best regards,
--  
Tal Zussman <tz2294@columbia.edu>

[PATCH 1/2] iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec

[Tal Zussman]

iommu_fwspec_init() allocates dev->iommu with dev_iommu_get() and then
the fwspec. If the fwspec allocation fails it returns -ENOMEM but leaves
dev->iommu allocated, and it leaks.

kmemleak reports it when iommufd_fail_nth injects the allocation
failure:

  unreferenced object 0xffff88811c138b00 (size 192):
    comm "iommufd_fail_nt", pid 783
    backtrace:
      __kmalloc_cache_noprof
      dev_iommu_get
      iommu_fwspec_init
      iommu_mock_device_add
      iommufd_test
      iommufd_fops_ioctl

Free dev->iommu on failure if this call allocated it. iommu_init_device()
allocates it before calling into the driver and frees it on its own
error path, so leave it alone in that case.

Fixes: 72acd9df18f1 ("iommu: Move iommu_fwspec to struct dev_iommu")
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 drivers/iommu/iommu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cd1bca7ede9a..69be8819e05d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3074,6 +3074,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
 {
 	const struct iommu_device *iommu = iommu_from_fwnode(iommu_fwnode);
 	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	bool new_iommu = !dev->iommu;
 
 	if (!iommu)
 		return driver_deferred_probe_check_state(dev);
@@ -3088,8 +3089,11 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
 
 	/* Preallocate for the overwhelmingly common case of 1 ID */
 	fwspec = kzalloc_flex(*fwspec, ids, 1);
-	if (!fwspec)
+	if (!fwspec) {
+		if (new_iommu)
+			dev_iommu_free(dev);
 		return -ENOMEM;
+	}
 
 	fwnode_handle_get(iommu_fwnode);
 	fwspec->iommu_fwnode = iommu_fwnode;

-- 
2.39.5

Re: [PATCH 1/2] iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec

[Robin Murphy]

On 31/10/2026 8:33 pm, Tal Zussman wrote:
> iommu_fwspec_init() allocates dev->iommu with dev_iommu_get() and then
> the fwspec. If the fwspec allocation fails it returns -ENOMEM but leaves
> dev->iommu allocated, and it leaks.
> 
> kmemleak reports it when iommufd_fail_nth injects the allocation
> failure:
> 
>    unreferenced object 0xffff88811c138b00 (size 192):
>      comm "iommufd_fail_nt", pid 783
>      backtrace:
>        __kmalloc_cache_noprof
>        dev_iommu_get
>        iommu_fwspec_init
>        iommu_mock_device_add
>        iommufd_test
>        iommufd_fops_ioctl
> 
> Free dev->iommu on failure if this call allocated it. iommu_init_device()
> allocates it before calling into the driver and frees it on its own
> error path, so leave it alone in that case.
> 
> Fixes: 72acd9df18f1 ("iommu: Move iommu_fwspec to struct dev_iommu")

No, the bug is clearly the early return in 2a918911ed3d ("iommufd: 
Register iommufd mock devices with fwspec"), so fix it there please. 
iommu_fwspec_init() has always been designed to be called in conjunction 
with iommu_probe_device() which handles cleanup appropriately. All of 
this mock_device hackery not using the proper probe paths is enough of a 
pain as it is...

Thanks,
Robin.

> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>   drivers/iommu/iommu.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index cd1bca7ede9a..69be8819e05d 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3074,6 +3074,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>   {
>   	const struct iommu_device *iommu = iommu_from_fwnode(iommu_fwnode);
>   	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +	bool new_iommu = !dev->iommu;
>   
>   	if (!iommu)
>   		return driver_deferred_probe_check_state(dev);
> @@ -3088,8 +3089,11 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>   
>   	/* Preallocate for the overwhelmingly common case of 1 ID */
>   	fwspec = kzalloc_flex(*fwspec, ids, 1);
> -	if (!fwspec)
> +	if (!fwspec) {
> +		if (new_iommu)
> +			dev_iommu_free(dev);
>   		return -ENOMEM;
> +	}
>   
>   	fwnode_handle_get(iommu_fwnode);
>   	fwspec->iommu_fwnode = iommu_fwnode;
> 

Re: [PATCH 1/2] iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec

[Tal Zussman]

On 11/2/26 11:51 AM, Robin Murphy wrote:
> On 31/10/2026 8:33 pm, Tal Zussman wrote:
>> iommu_fwspec_init() allocates dev->iommu with dev_iommu_get() and then
>> the fwspec. If the fwspec allocation fails it returns -ENOMEM but leaves
>> dev->iommu allocated, and it leaks.
>> 
>> kmemleak reports it when iommufd_fail_nth injects the allocation
>> failure:
>> 
>>    unreferenced object 0xffff88811c138b00 (size 192):
>>      comm "iommufd_fail_nt", pid 783
>>      backtrace:
>>        __kmalloc_cache_noprof
>>        dev_iommu_get
>>        iommu_fwspec_init
>>        iommu_mock_device_add
>>        iommufd_test
>>        iommufd_fops_ioctl
>> 
>> Free dev->iommu on failure if this call allocated it. iommu_init_device()
>> allocates it before calling into the driver and frees it on its own
>> error path, so leave it alone in that case.
>> 
>> Fixes: 72acd9df18f1 ("iommu: Move iommu_fwspec to struct dev_iommu")
> 
> No, the bug is clearly the early return in 2a918911ed3d ("iommufd:
> Register iommufd mock devices with fwspec"), so fix it there please.
> iommu_fwspec_init() has always been designed to be called in conjunction
> with iommu_probe_device() which handles cleanup appropriately. All of
> this mock_device hackery not using the proper probe paths is enough of a
> pain as it is...
> 
> Thanks,
> Robin.
> 

Thanks for the review!
I'll change this for v2.

- Tal

>> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
>> ---
>>   drivers/iommu/iommu.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index cd1bca7ede9a..69be8819e05d 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -3074,6 +3074,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>>   {
>>   	const struct iommu_device *iommu = iommu_from_fwnode(iommu_fwnode);
>>   	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> +	bool new_iommu = !dev->iommu;
>>   
>>   	if (!iommu)
>>   		return driver_deferred_probe_check_state(dev);
>> @@ -3088,8 +3089,11 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>>   
>>   	/* Preallocate for the overwhelmingly common case of 1 ID */
>>   	fwspec = kzalloc_flex(*fwspec, ids, 1);
>> -	if (!fwspec)
>> +	if (!fwspec) {
>> +		if (new_iommu)
>> +			dev_iommu_free(dev);
>>   		return -ENOMEM;
>> +	}
>>   
>>   	fwnode_handle_get(iommu_fwnode);
>>   	fwspec->iommu_fwnode = iommu_fwnode;
>> 
> 

Re: [PATCH 1/2] iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec

[Jason Gunthorpe]

On Mon, Nov 02, 2026 at 04:51:05PM +0000, Robin Murphy wrote:
> On 31/10/2026 8:33 pm, Tal Zussman wrote:
> > iommu_fwspec_init() allocates dev->iommu with dev_iommu_get() and then
> > the fwspec. If the fwspec allocation fails it returns -ENOMEM but leaves
> > dev->iommu allocated, and it leaks.
> > 
> > kmemleak reports it when iommufd_fail_nth injects the allocation
> > failure:
> > 
> >    unreferenced object 0xffff88811c138b00 (size 192):
> >      comm "iommufd_fail_nt", pid 783
> >      backtrace:
> >        __kmalloc_cache_noprof
> >        dev_iommu_get
> >        iommu_fwspec_init
> >        iommu_mock_device_add
> >        iommufd_test
> >        iommufd_fops_ioctl
> > 
> > Free dev->iommu on failure if this call allocated it. iommu_init_device()
> > allocates it before calling into the driver and frees it on its own
> > error path, so leave it alone in that case.
> > 
> > Fixes: 72acd9df18f1 ("iommu: Move iommu_fwspec to struct dev_iommu")
> 
> No, the bug is clearly the early return in 2a918911ed3d ("iommufd: Register
> iommufd mock devices with fwspec"), so fix it there please.
> iommu_fwspec_init() has always been designed to be called in conjunction
> with iommu_probe_device() which handles cleanup appropriately.

Asking AI to audit this found the other case I remember...

Yes: ACPI DMA configuration during driver binding can leave dev->iommu
allocated after failure:

 really_probe()
  -> dev->bus->dma_configure()
    -> acpi_dma_configure_id()
      -> acpi_iommu_configure_id()
        -> IORT / RIMT / VIOT
          -> acpi_iommu_fwspec_init()
            -> iommu_fwspec_init()

 If dev_iommu_get() allocates successfully and the subsequent fwspec
 allocation fails, acpi_iommu_configure_id() (line 1614) does no
 cleanup. acpi_dma_configure_id() (line 1670) ignores errors other than
 -EPROBE_DEFER. This invocation has no enclosing iommu_init_device() to
 unwind it.

The OF path has its own manual free fix already, it doesn't rely on
iommu_probe_device() to clean it. The other driver users are the only
things actually protected by ops->probe_device().

> > @@ -3074,6 +3074,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
> >   {
> >   	const struct iommu_device *iommu = iommu_from_fwnode(iommu_fwnode);
> >   	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +	bool new_iommu = !dev->iommu;
> >   	if (!iommu)
> >   		return driver_deferred_probe_check_state(dev);
> > @@ -3088,8 +3089,11 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
> >   	/* Preallocate for the overwhelmingly common case of 1 ID */
> >   	fwspec = kzalloc_flex(*fwspec, ids, 1);
> > -	if (!fwspec)
> > +	if (!fwspec) {
> > +		if (new_iommu)
> > +			dev_iommu_free(dev);
> >   		return -ENOMEM;
> > +	}

Just move this allocation before the dev_iommu_get() and free it if
the get fails. Then the function has a more sensible calling
convention that error means it doesn't need any unwinding. The
new_iommu is a little too weird.

Jason

Re: [PATCH 1/2] iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec

[Robin Murphy]

On 2026-11-03 7:32 pm, Jason Gunthorpe wrote:
> On Mon, Nov 02, 2026 at 04:51:05PM +0000, Robin Murphy wrote:
>> On 31/10/2026 8:33 pm, Tal Zussman wrote:
>>> iommu_fwspec_init() allocates dev->iommu with dev_iommu_get() and then
>>> the fwspec. If the fwspec allocation fails it returns -ENOMEM but leaves
>>> dev->iommu allocated, and it leaks.
>>>
>>> kmemleak reports it when iommufd_fail_nth injects the allocation
>>> failure:
>>>
>>>     unreferenced object 0xffff88811c138b00 (size 192):
>>>       comm "iommufd_fail_nt", pid 783
>>>       backtrace:
>>>         __kmalloc_cache_noprof
>>>         dev_iommu_get
>>>         iommu_fwspec_init
>>>         iommu_mock_device_add
>>>         iommufd_test
>>>         iommufd_fops_ioctl
>>>
>>> Free dev->iommu on failure if this call allocated it. iommu_init_device()
>>> allocates it before calling into the driver and frees it on its own
>>> error path, so leave it alone in that case.
>>>
>>> Fixes: 72acd9df18f1 ("iommu: Move iommu_fwspec to struct dev_iommu")
>>
>> No, the bug is clearly the early return in 2a918911ed3d ("iommufd: Register
>> iommufd mock devices with fwspec"), so fix it there please.
>> iommu_fwspec_init() has always been designed to be called in conjunction
>> with iommu_probe_device() which handles cleanup appropriately.
> 
> Asking AI to audit this found the other case I remember...
> 
> Yes: ACPI DMA configuration during driver binding can leave dev->iommu
> allocated after failure:
> 
>   really_probe()
>    -> dev->bus->dma_configure()
>      -> acpi_dma_configure_id()
>        -> acpi_iommu_configure_id()
>          -> IORT / RIMT / VIOT
>            -> acpi_iommu_fwspec_init()
>              -> iommu_fwspec_init()
> 
>   If dev_iommu_get() allocates successfully and the subsequent fwspec
>   allocation fails, acpi_iommu_configure_id() (line 1614) does no
>   cleanup. acpi_dma_configure_id() (line 1670) ignores errors other than
>   -EPROBE_DEFER. This invocation has no enclosing iommu_init_device() to
>   unwind it.

Yes, because that path will no longer allocate - if we reach 
iommu_fwspec_init() from the really_probe path at all now it's because 
the IOMMU is not yet registered, thus we'll always take the early return.

In fact I think the only reason to still have dev_iommu_get() in 
iommu_fwspec_init(), rather than just dereferencing dev->iommu directly, 
is for the sake of those few remaining drivers which abuse 
of_dma_configure() directly that I'm not sure what to do about.

Thanks,
Robin.
> The OF path has its own manual free fix already, it doesn't rely on
> iommu_probe_device() to clean it. The other driver users are the only
> things actually protected by ops->probe_device().
> 
>>> @@ -3074,6 +3074,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>>>    {
>>>    	const struct iommu_device *iommu = iommu_from_fwnode(iommu_fwnode);
>>>    	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>>> +	bool new_iommu = !dev->iommu;
>>>    	if (!iommu)
>>>    		return driver_deferred_probe_check_state(dev);
>>> @@ -3088,8 +3089,11 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>>>    	/* Preallocate for the overwhelmingly common case of 1 ID */
>>>    	fwspec = kzalloc_flex(*fwspec, ids, 1);
>>> -	if (!fwspec)
>>> +	if (!fwspec) {
>>> +		if (new_iommu)
>>> +			dev_iommu_free(dev);
>>>    		return -ENOMEM;
>>> +	}
> 
> Just move this allocation before the dev_iommu_get() and free it if
> the get fails. Then the function has a more sensible calling
> convention that error means it doesn't need any unwinding. The
> new_iommu is a little too weird.
> 
> Jason

[PATCH 2/2] iommufd: copy to and from folios in batch_rw()

[Tal Zussman]

batch_rw() copies one page at a time and, for writes, dirties each page
with set_page_dirty_lock(). Walk the pfn batch a folio at a time
instead, copying with memcpy_to_folio() and memcpy_from_folio() and
marking each folio dirty once. Convert the single-page copy in
iopt_pages_rw_page() to the same helper.

iopt_pages_rw_access() rejects dmabuf pages, so only pinned user memory
and memfd pages can reach batch_rw(), and those are always folios.

memcpy_to_folio() also flushes the dcache after the write, which the
open-coded memcpy() didn't.

This removes the last use of set_page_dirty_lock() in iommufd.

With ftrace timing iopt_pages_rw_slow() through the mock driver on a
non-debug kernel, slow-path writes to THP-backed pages drop from ~1310 to
~930 ns/page (now the same cost as a read), and writes to 4K folios drop
~9%.

Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 drivers/iommu/iommufd/pages.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
index 2f6153108add..133fed4495dd 100644
--- a/drivers/iommu/iommufd/pages.c
+++ b/drivers/iommu/iommufd/pages.c
@@ -721,19 +721,15 @@ static void batch_unpin(struct pfn_batch *batch, struct iopt_pages *pages,
 	}
 }
 
-static void copy_data_page(struct page *page, void *data, unsigned long offset,
-			   size_t length, unsigned int flags)
+static void copy_data_folio(struct folio *folio, size_t offset, void *data,
+			    size_t length, unsigned int flags)
 {
-	void *mem;
-
-	mem = kmap_local_page(page);
 	if (flags & IOMMUFD_ACCESS_RW_WRITE) {
-		memcpy(mem + offset, data, length);
-		set_page_dirty_lock(page);
+		memcpy_to_folio(folio, offset, data, length);
+		folio_mark_dirty_lock(folio);
 	} else {
-		memcpy(data, mem + offset, length);
+		memcpy_from_folio(data, folio, offset, length);
 	}
-	kunmap_local(mem);
 }
 
 static unsigned long batch_rw(struct pfn_batch *batch, void *data,
@@ -745,15 +741,21 @@ static unsigned long batch_rw(struct pfn_batch *batch, void *data,
 	unsigned int cur = 0;
 
 	while (cur < batch->end) {
-		unsigned long bytes = min(length, PAGE_SIZE - offset);
-
-		copy_data_page(pfn_to_page(batch->pfns[cur] + npage), data,
-			       offset, bytes, flags);
+		unsigned long pfn = batch->pfns[cur] + npage;
+		struct folio *folio = pfn_folio(pfn);
+		unsigned long idx = pfn - folio_pfn(folio);
+		unsigned long nr = min_t(unsigned long,
+					 folio_nr_pages(folio) - idx,
+					 batch->npfns[cur] - npage);
+		unsigned long bytes = min(length, nr * PAGE_SIZE - offset);
+
+		copy_data_folio(folio, idx * PAGE_SIZE + offset, data, bytes,
+				flags);
 		offset = 0;
 		length -= bytes;
 		data += bytes;
 		copied += bytes;
-		npage++;
+		npage += nr;
 		if (npage == batch->npfns[cur]) {
 			npage = 0;
 			cur++;
@@ -2306,6 +2308,7 @@ static int iopt_pages_rw_page(struct iopt_pages *pages, unsigned long index,
 			      unsigned long length, unsigned int flags)
 {
 	struct page *page = NULL;
+	struct folio *folio;
 	int rc;
 
 	if (IS_ENABLED(CONFIG_IOMMUFD_TEST) &&
@@ -2332,7 +2335,9 @@ static int iopt_pages_rw_page(struct iopt_pages *pages, unsigned long index,
 			rc = -EINVAL;
 		goto out_mmput;
 	}
-	copy_data_page(page, data, offset, length, flags);
+	folio = page_folio(page);
+	copy_data_folio(folio, folio_page_idx(folio, page) * PAGE_SIZE + offset,
+			data, length, flags);
 	unpin_user_page(page);
 	rc = 0;
 

-- 
2.39.5

Re: [PATCH 2/2] iommufd: copy to and from folios in batch_rw()

[Jason Gunthorpe]

On Sat, Oct 31, 2026 at 04:33:19PM -0400, Tal Zussman wrote:
> batch_rw() copies one page at a time and, for writes, dirties each page
> with set_page_dirty_lock(). Walk the pfn batch a folio at a time
> instead, copying with memcpy_to_folio() and memcpy_from_folio() and
> marking each folio dirty once. Convert the single-page copy in
> iopt_pages_rw_page() to the same helper.
> 
> iopt_pages_rw_access() rejects dmabuf pages, so only pinned user memory
> and memfd pages can reach batch_rw(), and those are always folios.
> 
> memcpy_to_folio() also flushes the dcache after the write, which the
> open-coded memcpy() didn't.
> 
> This removes the last use of set_page_dirty_lock() in iommufd.
> 
> With ftrace timing iopt_pages_rw_slow() through the mock driver on a
> non-debug kernel, slow-path writes to THP-backed pages drop from ~1310 to
> ~930 ns/page (now the same cost as a read), and writes to 4K folios drop
> ~9%.
> 
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>  drivers/iommu/iommufd/pages.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)

This looks reasonable to me

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason