[PATCH] iommufd: take dma_resv lock before dma_buf_unpin() in release path

[PATCH] iommufd: take dma_resv lock before dma_buf_unpin() in release path

[Ankit Soni]

dma_buf_unpin() requires the caller to hold the exporter's dma_resv
lock:

  void dma_buf_unpin(struct dma_buf_attachment *attach)
  {
          ...
          dma_resv_assert_held(dmabuf->resv);
          ...
  }

iopt_release_pages() calls dma_buf_unpin() without taking that lock,
so every iommufd_ioas_destroy()/iommufd_ioas_unmap() that releases
the last reference on a DMABUF-backed iopt_pages triggers a WARN.
This was hit while running tools/testing/selftests/iommu/iommufd:

  WARNING: drivers/dma-buf/dma-buf.c:1137 at dma_buf_unpin+0x62/0x70
  RIP: 0010:dma_buf_unpin+0x62/0x70
  Call Trace:
   <TASK>
   dma_buf_unpin+0x62/0x70
   iopt_release_pages+0xe4/0x190
   iopt_unmap_iova_range+0x1c7/0x290
   iopt_unmap_all+0x1a/0x30
   iommufd_ioas_destroy+0x1d/0x50
   iommufd_fops_release+0x93/0x150
   __fput+0xfc/0x2c0
   __x64_sys_close+0x3d/0x80
   do_syscall_64+0x65/0x180
   </TASK>

Take the dma_resv lock around dma_buf_unpin() in iopt_release_pages(),
matching the iopt_map_dmabuf() convention. dma_buf_detach() acquires the
reservation lock internally, so it must remain outside the locked region.

Fixes: 8c5f9645c389 ("iommufd: Add dma_buf_pin()")
Reported-by: Ankit Soni <Ankit.Soni@amd.com>
Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
---
 drivers/iommu/iommufd/pages.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
index 9bdb2945afe1..7b64002e54b9 100644
--- a/drivers/iommu/iommufd/pages.c
+++ b/drivers/iommu/iommufd/pages.c
@@ -1663,7 +1663,9 @@ void iopt_release_pages(struct kref *kref)
 	if (iopt_is_dmabuf(pages) && pages->dmabuf.attach) {
 		struct dma_buf *dmabuf = pages->dmabuf.attach->dmabuf;
 
+		dma_resv_lock(dmabuf->resv, NULL);
 		dma_buf_unpin(pages->dmabuf.attach);
+		dma_resv_unlock(dmabuf->resv);
 		dma_buf_detach(dmabuf, pages->dmabuf.attach);
 		dma_buf_put(dmabuf);
 		WARN_ON(!list_empty(&pages->dmabuf.tracker));
-- 
2.43.0

Re: [PATCH] iommufd: take dma_resv lock before dma_buf_unpin() in release path

[Jason Gunthorpe]

On Tue, May 26, 2026 at 11:10:34AM +0000, Ankit Soni wrote:
> dma_buf_unpin() requires the caller to hold the exporter's dma_resv
> lock:
> 
>   void dma_buf_unpin(struct dma_buf_attachment *attach)
>   {
>           ...
>           dma_resv_assert_held(dmabuf->resv);
>           ...
>   }
> 
> iopt_release_pages() calls dma_buf_unpin() without taking that lock,
> so every iommufd_ioas_destroy()/iommufd_ioas_unmap() that releases
> the last reference on a DMABUF-backed iopt_pages triggers a WARN.
> This was hit while running tools/testing/selftests/iommu/iommufd:

Any idea why this is comming up now? Did I run the tests without some
kind of debug option to turn on that assertion maybe?

Jason

Re: [PATCH] iommufd: take dma_resv lock before dma_buf_unpin() in release path

[Ankit Soni]

On Tue, May 26, 2026 at 09:26:56AM -0300, Jason Gunthorpe wrote:
> On Tue, May 26, 2026 at 11:10:34AM +0000, Ankit Soni wrote:
> > dma_buf_unpin() requires the caller to hold the exporter's dma_resv
> > lock:
> > 
> >   void dma_buf_unpin(struct dma_buf_attachment *attach)
> >   {
> >           ...
> >           dma_resv_assert_held(dmabuf->resv);
> >           ...
> >   }
> > 
> > iopt_release_pages() calls dma_buf_unpin() without taking that lock,
> > so every iommufd_ioas_destroy()/iommufd_ioas_unmap() that releases
> > the last reference on a DMABUF-backed iopt_pages triggers a WARN.
> > This was hit while running tools/testing/selftests/iommu/iommufd:
> 
> Any idea why this is comming up now? Did I run the tests without some
> kind of debug option to turn on that assertion maybe?
> 
> Jason

The assertion is gated by CONFIG_LOCKDEP. My config has it on via
CONFIG_DEBUG_LOCK_ALLOC=y (LOCK_STAT, PROVE_LOCKING and
DEBUG_WW_MUTEX_SLOWPATH each select-chain to LOCKDEP as well).

-Ankit

Re: [PATCH] iommufd: take dma_resv lock before dma_buf_unpin() in release path

[Jason Gunthorpe]

On Tue, May 26, 2026 at 11:10:34AM +0000, Ankit Soni wrote:

> Take the dma_resv lock around dma_buf_unpin() in iopt_release_pages(),
> matching the iopt_map_dmabuf() convention. dma_buf_detach() acquires the
> reservation lock internally, so it must remain outside the locked region.
> 
> Fixes: 8c5f9645c389 ("iommufd: Add dma_buf_pin()")
> Reported-by: Ankit Soni <Ankit.Soni@amd.com>
> Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
> ---
>  drivers/iommu/iommufd/pages.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied to for-next

Thanks,
Jason