public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] dma-buf: fix order of trace and fput
@ 2026-04-08 12:24 Christian König
  2026-04-09 12:38 ` Andi Shyti
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Christian König @ 2026-04-08 12:24 UTC (permalink / raw)
  To: janusz.krzysztofik, dri-devel, linaro-mm-sig, linux-media,
	gaoxiang17
  Cc: sumit.semwal

Dropping the last reference to the DMA-buf and then accessing the name
for tracing can't work correctly.

Only compile tested!

Signed-off-by: Christian König <christian.koenig@amd.com>
Fixed: 281a22631423 ("dma-buf: add some tracepoints to debug.")
---
 drivers/dma-buf/dma-buf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 11711874a325..3a9d5113b98c 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -845,9 +845,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
 	if (WARN_ON(!dmabuf || !dmabuf->file))
 		return;
 
-	fput(dmabuf->file);
-
 	DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
+	fput(dmabuf->file);
 }
 EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
 
-- 
2.43.0


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

* Re: [PATCH] dma-buf: fix order of trace and fput
  2026-04-08 12:24 [PATCH] dma-buf: fix order of trace and fput Christian König
@ 2026-04-09 12:38 ` Andi Shyti
  2026-04-12  2:31 ` Claude review: " Claude Code Review Bot
  2026-04-12  2:31 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Andi Shyti @ 2026-04-09 12:38 UTC (permalink / raw)
  To: Christian König
  Cc: janusz.krzysztofik, dri-devel, linaro-mm-sig, linux-media,
	gaoxiang17, sumit.semwal

Hi Christian,

> @@ -845,9 +845,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
>  	if (WARN_ON(!dmabuf || !dmabuf->file))
>  		return;
>  
> -	fput(dmabuf->file);
> -
>  	DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
> +	fput(dmabuf->file);

funny, I just found out I sent the exact same patch, just few
minutes later :-) [*]

Andi

>  }

[*] https://lore.kernel.org/all/20260408123916.2604101-2-andi.shyti@kernel.org/

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

* Claude review: dma-buf: fix order of trace and fput
  2026-04-08 12:24 [PATCH] dma-buf: fix order of trace and fput Christian König
  2026-04-09 12:38 ` Andi Shyti
@ 2026-04-12  2:31 ` Claude Code Review Bot
  2026-04-12  2:31 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  2:31 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dma-buf: fix order of trace and fput
Author: "=?UTF-8?q?Christian=20K=C3=B6nig?=" <ckoenig.leichtzumerken@gmail.com>
Patches: 2
Reviewed: 2026-04-12T12:31:04.964930

---

This is a single-patch fix for a use-after-free bug in `dma_buf_put()`. The bug is real and the fix is correct: after `fput(dmabuf->file)` drops the last reference, the dma-buf may be freed, yet the current code proceeds to call `DMA_BUF_TRACE(trace_dma_buf_put, dmabuf)` which accesses `dmabuf->name_lock`, `dmabuf->exp_name`, `dmabuf->size`, and `dmabuf->file->f_inode->i_ino` — all potentially freed memory.

The fix simply reorders the two operations so the tracepoint fires before `fput()`. This is the obvious and correct approach.

There is one tag issue that should be fixed before merging.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: dma-buf: fix order of trace and fput
  2026-04-08 12:24 [PATCH] dma-buf: fix order of trace and fput Christian König
  2026-04-09 12:38 ` Andi Shyti
  2026-04-12  2:31 ` Claude review: " Claude Code Review Bot
@ 2026-04-12  2:31 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  2:31 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**The bug (confirmed):** The `DMA_BUF_TRACE` macro (defined at `drivers/dma-buf/dma-buf.c:47`) acquires `dmabuf->name_lock` and then the tracepoint itself accesses `dmabuf->exp_name`, `dmabuf->size`, and `dmabuf->file->f_inode->i_ino`. Calling this after `fput()` is a use-after-free if that `fput()` drops the last reference, since the file's release callback will free the dma-buf.

**The fix is correct:**

```diff
-	fput(dmabuf->file);
-
 	DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
+	fput(dmabuf->file);
```

Moving the trace before `fput()` ensures the dmabuf and its file are still alive when the tracepoint accesses their fields. The `WARN_ON(!dmabuf || !dmabuf->file)` guard above already validates both pointers before we reach this code.

**Tag issue:** The commit message uses `Fixed:` but the standard kernel tag is `Fixes:` (with an 's'). This should be corrected:

```
Fixed: 281a22631423 ("dma-buf: add some tracepoints to debug.")
```
should be:
```
Fixes: 281a22631423 ("dma-buf: add some tracepoints to debug.")
```

Scripts like `scripts/checkpatch.pl` and automated stable-kernel backport tooling rely on the `Fixes:` tag being spelled correctly.

**Minor note:** The commit message says "Only compile tested!" — this is appreciated transparency, but given how straightforward the reordering is, the fix is clearly correct by inspection. The tracepoint just needs to read fields before they're potentially freed; there's no subtle ordering dependency in the other direction (the trace doesn't need to observe post-fput state).

**Verdict:** The fix itself is good — one clear use-after-free fixed with a minimal, obviously-correct reordering. Just fix `Fixed:` → `Fixes:` before applying.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-04-12  2:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 12:24 [PATCH] dma-buf: fix order of trace and fput Christian König
2026-04-09 12:38 ` Andi Shyti
2026-04-12  2:31 ` Claude review: " Claude Code Review Bot
2026-04-12  2:31 ` 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