public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] dma-buf: fix UAF in dma_buf_fd() tracepoint
@ 2026-05-23 18:14 David Carlier
  2026-05-25  7:28 ` Claude review: " Claude Code Review Bot
  2026-05-25  7:28 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: David Carlier @ 2026-05-23 18:14 UTC (permalink / raw)
  To: Sumit Semwal, Christian König
  Cc: gaoxiang17, linux-media, dri-devel, linaro-mm-sig, linux-kernel,
	syzbot+7f4987d0afb97dd090cb, David Carlier, stable

Once FD_ADD() returns, the fd is live in the file descriptor table
and a thread sharing that table can close() it before DMA_BUF_TRACE()
runs. The close drops the last reference, __fput() frees the dma_buf,
and the tracepoint then dereferences dmabuf to take dmabuf->name_lock
-- slab-use-after-free.

Split FD_ADD() back into get_unused_fd_flags() + fd_install() and
emit the tracepoint between them. While the fdtable slot is reserved
with a NULL file pointer, a racing close() returns -EBADF without
entering __fput(), so the dma_buf stays alive across the trace. Same
approach as commit 2d76319c4cbb ("dma-buf: fix UAF in dma_buf_put()
tracepoint").

This undoes the FD_ADD() conversion done in commit 34dfce523c90
("dma: convert dma_buf_fd() to FD_ADD()"); FD_ADD() has no place to
hook the tracepoint safely.

Reported-by: syzbot+7f4987d0afb97dd090cb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7f4987d0afb97dd090cb
Fixes: 281a22631423 ("dma-buf: add some tracepoints to debug.")
Cc: stable@vger.kernel.org # 7.0.x
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/dma-buf/dma-buf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 71f37544a5c6..d504c636dc29 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -792,9 +792,13 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags)
 	if (!dmabuf || !dmabuf->file)
 		return -EINVAL;
 
-	fd = FD_ADD(flags, dmabuf->file);
+	fd = get_unused_fd_flags(flags);
+	if (fd < 0)
+		return fd;
+
 	DMA_BUF_TRACE(trace_dma_buf_fd, dmabuf, fd);
 
+	fd_install(fd, dmabuf->file);
 	return fd;
 }
 EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: dma-buf: fix UAF in dma_buf_fd() tracepoint
  2026-05-23 18:14 [PATCH] dma-buf: fix UAF in dma_buf_fd() tracepoint David Carlier
@ 2026-05-25  7:28 ` Claude Code Review Bot
  2026-05-25  7:28 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:28 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dma-buf: fix UAF in dma_buf_fd() tracepoint
Author: David Carlier <devnexen@gmail.com>
Patches: 1
Reviewed: 2026-05-25T17:28:46.572678

---

This is a single-patch fix for a real UAF (use-after-free) bug in the dma-buf subsystem, reported by syzbot. The analysis is correct: `FD_ADD()` atomically allocates an fd and installs the file, meaning the fd is live and closeable before the `DMA_BUF_TRACE()` tracepoint runs. A racing `close()` on the fd can free the `dma_buf`, and then the tracepoint dereferences `dmabuf->name_lock` — slab-use-after-free.

The fix is sound: split `FD_ADD()` back into `get_unused_fd_flags()` + `fd_install()`, placing the tracepoint between them. While the fd slot is reserved but has a NULL file pointer, a concurrent `close()` returns `-EBADF` without entering `__fput()`, so the `dma_buf` remains alive during the trace. This is the same pattern already used for the `dma_buf_put()` tracepoint (commit 2d76319c4cbb).

The patch is clean, minimal, well-explained, and appropriate for stable backport. No concerns.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: dma-buf: fix UAF in dma_buf_fd() tracepoint
  2026-05-23 18:14 [PATCH] dma-buf: fix UAF in dma_buf_fd() tracepoint David Carlier
  2026-05-25  7:28 ` Claude review: " Claude Code Review Bot
@ 2026-05-25  7:28 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.**

The race condition is real and the fix is correct. Looking at the `DMA_BUF_TRACE` macro (line 47-54 of `dma-buf.c`):

```c
#define DMA_BUF_TRACE(FUNC, ...)					\
	do {								\
		if (IS_ENABLED(CONFIG_LOCKDEP) || FUNC##_enabled()) {	\
			guard(spinlock)(&dmabuf->name_lock);		\
			FUNC(__VA_ARGS__);				\
		}							\
	} while (0)
```

This takes `dmabuf->name_lock`, so `dmabuf` must be alive when the tracepoint fires. With `FD_ADD()`, the file is already installed and visible to other threads, so a racing `close()` can trigger `__fput()` → release → free before the tracepoint runs.

The fix correctly places the tracepoint in the window between `get_unused_fd_flags()` (fd reserved, not yet populated) and `fd_install()` (fd becomes live):

```c
fd = get_unused_fd_flags(flags);
if (fd < 0)
    return fd;

DMA_BUF_TRACE(trace_dma_buf_fd, dmabuf, fd);

fd_install(fd, dmabuf->file);
```

**Error handling: Correct.** The `fd < 0` check properly handles `get_unused_fd_flags()` failure and returns the error. No file reference leak since `dmabuf->file` hasn't been consumed yet at that point.

**Consistency:** This matches the pattern used by `dma_buf_put()` at line 852, where the tracepoint fires before `fput()` to avoid the same class of UAF.

**Commit message: Excellent.** Clear description of the race, the fix mechanism, references to the prior analogous fix, proper Fixes/Cc-stable/Reported-by tags, and correctly identifies the original commit that introduced the tracepoints as the Fixes target rather than the `FD_ADD()` conversion (since the bug predates `FD_ADD()` — it just became more visible with it).

**Stable tag:** `Cc: stable@vger.kernel.org # 7.0.x` — appropriate since the `FD_ADD()` conversion happened recently and this is the tree where the regression manifests.

**No issues found.** This is a clean, correct fix ready for merge.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-25  7:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-23 18:14 [PATCH] dma-buf: fix UAF in dma_buf_fd() tracepoint David Carlier
2026-05-25  7:28 ` Claude review: " Claude Code Review Bot
2026-05-25  7:28 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox