public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] udmabuf: fix DMA direction mismatch in release_udmabuf()
@ 2026-03-14 23:27 Mikhail Gavrilov
  2026-03-16  1:52 ` Claude review: " Claude Code Review Bot
  2026-03-16  1:52 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Mikhail Gavrilov @ 2026-03-14 23:27 UTC (permalink / raw)
  To: kraxel, vivek.kasireddy
  Cc: sumit.semwal, christian.koenig, dri-devel, linux-media,
	linaro-mm-sig, stable, Mikhail Gavrilov

begin_cpu_udmabuf() maps the sg_table with the caller-provided direction
(e.g., DMA_TO_DEVICE for a write-only sync), and caches it in ubuf->sg
for reuse.  However, release_udmabuf() always unmaps this sg_table with
a hardcoded DMA_BIDIRECTIONAL, regardless of the direction that was
originally used for the mapping.

With CONFIG_DMA_API_DEBUG=y this produces:

  DMA-API: misc udmabuf: device driver frees DMA memory with different
  direction [device address=0x000000044a123000] [size=4096 bytes]
  [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The issue was found during video playback when GStreamer performed a
write-only DMA_BUF_IOCTL_SYNC on a udmabuf.  It can be reproduced
with CONFIG_DMA_API_DEBUG=y by creating a udmabuf from a memfd,
performing a write-only sync (DMA_BUF_SYNC_WRITE without
DMA_BUF_SYNC_READ), and closing the file descriptor.

Fix this by storing the DMA direction used when the sg_table is first
created in begin_cpu_udmabuf(), and passing that same direction to
put_sg_table() in release_udmabuf().

Fixes: 284562e1f348 ("udmabuf: implement begin_cpu_access/end_cpu_access hooks")
Cc: stable@vger.kernel.org
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
 drivers/dma-buf/udmabuf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 94b8ecb892bb..d0836febefdd 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -40,6 +40,7 @@ struct udmabuf {
 	struct folio **pinned_folios;
 
 	struct sg_table *sg;
+	enum dma_data_direction sg_dir;
 	struct miscdevice *device;
 	pgoff_t *offsets;
 };
@@ -235,7 +236,7 @@ static void release_udmabuf(struct dma_buf *buf)
 	struct device *dev = ubuf->device->this_device;
 
 	if (ubuf->sg)
-		put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL);
+		put_sg_table(dev, ubuf->sg, ubuf->sg_dir);
 
 	deinit_udmabuf(ubuf);
 	kfree(ubuf);
@@ -253,6 +254,8 @@ static int begin_cpu_udmabuf(struct dma_buf *buf,
 		if (IS_ERR(ubuf->sg)) {
 			ret = PTR_ERR(ubuf->sg);
 			ubuf->sg = NULL;
+		} else {
+			ubuf->sg_dir = direction;
 		}
 	} else {
 		dma_sync_sgtable_for_cpu(dev, ubuf->sg, direction);
-- 
2.53.0


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

* Claude review: udmabuf: fix DMA direction mismatch in release_udmabuf()
  2026-03-14 23:27 [PATCH] udmabuf: fix DMA direction mismatch in release_udmabuf() Mikhail Gavrilov
@ 2026-03-16  1:52 ` Claude Code Review Bot
  2026-03-16  1:52 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-16  1:52 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: udmabuf: fix DMA direction mismatch in release_udmabuf()
Author: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Patches: 1
Reviewed: 2026-03-16T11:52:22.902903

---

This is a single-patch fix for a real DMA API direction mismatch bug in `udmabuf`. The bug is clearly described: `begin_cpu_udmabuf()` maps the sg_table with the caller-provided direction but `release_udmabuf()` always unmapped with `DMA_BIDIRECTIONAL`, triggering a warning with `CONFIG_DMA_API_DEBUG=y`. The fix is minimal and correct — it stores the mapping direction and replays it on unmap.

**Verdict: Looks good overall, with one observation about a pre-existing issue worth noting.**

---
Generated by Claude Code Patch Reviewer

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

* Claude review: udmabuf: fix DMA direction mismatch in release_udmabuf()
  2026-03-14 23:27 [PATCH] udmabuf: fix DMA direction mismatch in release_udmabuf() Mikhail Gavrilov
  2026-03-16  1:52 ` Claude review: " Claude Code Review Bot
@ 2026-03-16  1:52 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-16  1:52 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Commit message:** Well-written. Clearly describes the problem, how to reproduce it, and the fix. The `Fixes:` tag and `Cc: stable` are appropriate.

**Code review:**

The new field addition is clean:

```c
struct sg_table *sg;
enum dma_data_direction sg_dir;
```

Since `ubuf` is allocated with `kzalloc()` (line 384), `sg_dir` is implicitly initialized to `0` which is `DMA_BIDIRECTIONAL`. This is fine because `sg_dir` is only read when `ubuf->sg != NULL`, and `sg_dir` is always set before `ubuf->sg` becomes non-NULL.

The fix in `release_udmabuf()` is correct:

```c
if (ubuf->sg)
    put_sg_table(dev, ubuf->sg, ubuf->sg_dir);
```

The direction is properly stored only on successful mapping:

```c
} else {
    ubuf->sg_dir = direction;
}
```

**One observation (pre-existing, not introduced by this patch):** In `begin_cpu_udmabuf()`, when `ubuf->sg` already exists (the `else` branch at line 260-261), `dma_sync_sgtable_for_cpu()` is called with the new `direction` parameter, which could differ from the direction the sg_table was originally mapped with. Strictly speaking, the DMA API requires sync operations to use the same direction as the original mapping. For example, if the first call maps with `DMA_TO_DEVICE` and a second call tries to sync with `DMA_FROM_DEVICE`, that would be an API misuse. This is a pre-existing issue, not caused by this patch, but the author may want to consider it as a follow-up (e.g., either enforce that the direction matches, or remap with the new direction).

**No issues found with this patch itself.**

Reviewed-by assessment: The patch is correct and ready to be applied.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-03-16  1:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-14 23:27 [PATCH] udmabuf: fix DMA direction mismatch in release_udmabuf() Mikhail Gavrilov
2026-03-16  1:52 ` Claude review: " Claude Code Review Bot
2026-03-16  1:52 ` 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