On Mon, 2026-10-19 at 16:17 +0000, redducc wrote:
> dma_fence_signal_timestamp_locked() clears fence->ops on signalling when
> the ops have neither a release nor a wait callback, which is the case
> for drm_crtc_fence_ops.
> 
> dma_fence_driver_name() and dma_fence_timeline_name() load fence->ops,
> check the signalled bit and then call into the ops. If the fence
> signals between that check and the call, fence_to_crtc() rereads
> fence->ops, finds NULL and hits the BUG_ON(). This takes down any
> caller of SYNC_IOC_FILE_INFO on a CRTC out-fence, such as Android's
> surfaceflinger, which hit it on a Snapdragon 8+ Gen 1 phone (msm).
> 
> The CRTC behind the fence outlives it, so reading the name after the
> ops were cleared is fine. Only warn when the fence carries foreign ops.

@Christian:
We have now seen several bugs like these, which is one of the reasons
why I believe that RCU is the wrong solution for dma_fence.

I present another rework RFC soonish.

> 
> Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
> Assisted-by: LLM
> Signed-off-by: redducc <[email protected]>

Reviewed-by: Philipp Stanner <[email protected]>


The Signed tag btw. states that you have the legal right to push this
code upstream, and that you agree with the kernel's licensing. It's
uncommon to use with pseudonyms and lessens your personal accumulation
of prestige ;)


Philipp

> ---
>  drivers/gpu/drm/drm_crtc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 63ead8ba6756..1dbd0e9db536 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -158,7 +158,13 @@ static const struct dma_fence_ops drm_crtc_fence_ops;
>  
>  static struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
>  {
> -     BUG_ON(rcu_access_pointer(fence->ops) != &drm_crtc_fence_ops);
> +     const struct dma_fence_ops *ops = rcu_access_pointer(fence->ops);
> +
> +     /*
> +      * Signalling clears ops, and can race with a name lookup that already
> +      * picked our callbacks, so only a foreign ops pointer is a bug.
> +      */
> +     WARN_ON_ONCE(ops && ops != &drm_crtc_fence_ops);
>       return container_of(fence->extern_lock, struct drm_crtc, fence_lock);
>  }
>  
> 
> base-commit: 6a56f4f29711dd2da49f03a5b99ce53779e638d7

Reply via email to