[PATCH] dma-fence: Move signalling tracepoint to before ops detach

[PATCH] dma-fence: Move signalling tracepoint to before ops detach

[Tvrtko Ursulin]

Move the signalling tracepoint to before fence->ops are reset otherwise
tracepoint will dereference a null pointer.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
Cc: Christian König <christian.koenig@amd.com>
Cc: Philipp Stanner <phasta@kernel.org>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-fence.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 1826ba73094c..1c1eaecaf1b0 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -363,6 +363,8 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 				      &fence->flags)))
 		return;
 
+	trace_dma_fence_signaled(fence);
+
 	/*
 	 * When neither a release nor a wait operation is specified set the ops
 	 * pointer to NULL to allow the fence structure to become independent
@@ -377,7 +379,6 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 
 	fence->timestamp = timestamp;
 	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
-	trace_dma_fence_signaled(fence);
 
 	list_for_each_entry_safe(cur, tmp, &cb_list, node) {
 		INIT_LIST_HEAD(&cur->node);
-- 
2.52.0

Re: [PATCH] dma-fence: Move signalling tracepoint to before ops detach

[Philipp Stanner]

IMO the title should state that this fixes a NULL ptr deref, since that's very significant.

On Mon, 2026-03-30 at 14:36 +0100, Tvrtko Ursulin wrote:
> Move the signalling tracepoint to before fence->ops are reset otherwise
> tracepoint will dereference a null pointer.

Can't fully follow; you're talking about the fence ops detachment for
signaled fences?

> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Philipp Stanner <phasta@kernel.org>
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>  drivers/dma-buf/dma-fence.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 1826ba73094c..1c1eaecaf1b0 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -363,6 +363,8 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  				      &fence->flags)))
>  		return;
>  
> +	trace_dma_fence_signaled(fence);
> +

If it's about touching the ops, the decisive action would be the
IS_SIGNALED flag, wouldn't it? So trace_dma_fence_signaleld() should be
above the flag test?


P.

>  	/*
>  	 * When neither a release nor a wait operation is specified set the ops
>  	 * pointer to NULL to allow the fence structure to become independent
> @@ -377,7 +379,6 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  
>  	fence->timestamp = timestamp;
>  	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
> -	trace_dma_fence_signaled(fence);
>  
>  	list_for_each_entry_safe(cur, tmp, &cb_list, node) {
>  		INIT_LIST_HEAD(&cur->node);

Claude review: dma-fence: Move signalling tracepoint to before ops detach

[Claude Code Review Bot]

Overall Series Review

Subject: dma-fence: Move signalling tracepoint to before ops detach
Author: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Patches: 2
Reviewed: 2026-03-31T17:14:43.479696

---

This is a single-patch bugfix for a NULL pointer dereference introduced by commit 541c8f2468b9 ("dma-buf: detach fence ops on signal v3"). The fix is correct, straightforward, and clearly necessary. The tracepoint class at line 24 of `dma_fence.h` accesses `fence->ops->get_driver_name(fence)` and `fence->ops->get_timeline_name(fence)`, which will crash if `fence->ops` has been set to NULL by the ops detach logic added in the Fixes commit.

The patch is **good to merge** as-is.

---
Generated by Claude Code Patch Reviewer

Claude review: dma-fence: Move signalling tracepoint to before ops detach

[Claude Code Review Bot]

Patch Review

**Analysis:** The tracepoint `trace_dma_fence_signaled(fence)` uses the `dma_fence` event class which dereferences `fence->ops` to call `get_driver_name()` and `get_timeline_name()` (see `include/trace/events/dma_fence.h` lines 24-25):

```c
__string(driver, fence->ops->get_driver_name(fence))
__string(timeline, fence->ops->get_timeline_name(fence))
```

The commit being fixed (541c8f2468b9) added logic that sets `fence->ops` to NULL when the ops has no `release` or `wait` callback:

```c
ops = rcu_dereference_protected(fence->ops, true);
if (!ops->release && !ops->wait)
    RCU_INIT_POINTER(fence->ops, NULL);
```

With the tracepoint placed *after* this code (as in the current tree at line 380), enabling the tracepoint will cause a NULL dereference for any fence whose ops lack both `release` and `wait`.

Moving the tracepoint before the ops detach logic is the correct fix. The tracepoint still runs after the `DMA_FENCE_FLAG_SIGNALED_BIT` is set (which is the logical signal point), so it still accurately records the signal event.

**Minor observation:** The tracepoint is now also emitted before `fence->timestamp` is assigned. This means the tracepoint fires slightly earlier in the sequence — before timestamp assignment and before callbacks run — but the tracepoint doesn't record the timestamp so this is not a functional concern.

**Nit on commit message format:** The `Fixes:` tag should conventionally come before the `Signed-off-by`. This is a minor style point and not a blocking issue.

**Verdict:** Reviewed-by worthy. The fix is minimal, correct, and addresses a real crash.

---
Generated by Claude Code Patch Reviewer