Claude review: iommufd: take dma_resv lock before dma_buf_unpin() in release path

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Correctness: Good.**

The patch adds the missing `dma_resv_lock()`/`dma_resv_unlock()` bracket around `dma_buf_unpin()`:

```c
+		dma_resv_lock(dmabuf->resv, NULL);
 		dma_buf_unpin(pages->dmabuf.attach);
+		dma_resv_unlock(dmabuf->resv);
 		dma_buf_detach(dmabuf, pages->dmabuf.attach);
```

This is correct for several reasons:

1. **`dma_buf_unpin()` requires the lock** — confirmed by `dma_resv_assert_held(dmabuf->resv)` at `drivers/dma-buf/dma-buf.c:1136`.

2. **`dma_buf_detach()` is correctly kept outside the locked region** — it acquires `dma_resv_lock` internally (`dma-buf.c:1078`), so holding it around `dma_buf_detach()` would deadlock since `dma_resv_lock` is not recursive.

3. **Symmetry with `iopt_map_dmabuf()`** — the pin path at `pages.c:1495-1513` holds `dma_resv_lock` around `dma_buf_pin()` and releases it before `dma_buf_detach()` in the error path (`pages.c:1519-1523`). This patch mirrors that exact pattern.

4. **`dma_resv_lock()` return value** — `dma_resv_lock(resv, NULL)` with a NULL context cannot return `-EDEADLK` (that only occurs with ww_acquire_ctx-based contention management). It can only block or succeed, so ignoring the return value is acceptable here. This is consistent with how `iopt_map_dmabuf()` and `dma_buf_detach()` both call it.

5. **No ordering concern with `pages->mutex`** — the function is called from `kref_put`, meaning the refcount has hit zero. No other code path can hold `pages->mutex` for this object simultaneously, so there's no lock ordering issue in practice. The lockdep annotation in `iopt_map_dmabuf()` (taking `pages->mutex` inside `dma_resv_lock`) is satisfied trivially since the mutex isn't held here.

**Minor observations (not blocking):**

- The `Fixes:` tag references `8c5f9645c389 ("iommufd: Add dma_buf_pin()")` which is appropriate — that's the commit that introduced the `dma_buf_unpin()` call in this path without the required locking.

- The patch has both `Reported-by` and `Signed-off-by` from the same author, which is fine — the author found and fixed the bug.

- A `Cc: stable@vger.kernel.org` tag could be appropriate given the `Fixes:` tag, but the maintainer can add that at merge time.

**No issues found. The fix is minimal, correct, and well-justified.**

---
Generated by Claude Code Patch Reviewer