[PATCH] drm: Don't BUG when a CRTC fence signals during a name lookup

[PATCH] drm: Don't BUG when a CRTC fence signals during a name lookup

[redducc]

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.

Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
Assisted-by: LLM
Signed-off-by: redducc <sushmit718@gmail.com>
---
 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
-- 
2.43.0

Re: [PATCH] drm: Don't BUG when a CRTC fence signals during a name lookup

[Philipp Stanner]

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 <sushmit718@gmail.com>

Reviewed-by: Philipp Stanner <phasta@kernel.org>


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

Re: [PATCH] drm: Don't BUG when a CRTC fence signals during a name lookup

[Tvrtko Ursulin]

On 20/10/2026 07:53, Philipp Stanner wrote:
> 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 <sushmit718@gmail.com>
> 
> Reviewed-by: Philipp Stanner <phasta@kernel.org>

Hm this looked familiar so I looked around. There was an earlier fix for 
the same from Andre with msg-id 
20260721-linux-drm_crtc_fix2-v3-2-afa8c71506e6@linaro.org which seems to 
have fallen through the cracks.

Regards,

Tvrtko

> 
> 
> 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

Re: [PATCH] drm: Don't BUG when a CRTC fence signals during a name lookup

[Philipp Stanner]

On Tue, 2026-10-20 at 07:57 +0100, Tvrtko Ursulin wrote:
> Hm this looked familiar so I looked around. There was an earlier fix for 
> the same from Andre with msg-id 
> 20260721-linux-drm_crtc_fix2-v3-2-afa8c71506e6@linaro.org which seems to 
> have fallen through the cracks.

Oh, right. Good catch.

patch №1 still misses an R-b.

Seems CRTC is one of the collectively maintained components. So one of
us has to push it.

I can do today, it synergies well with my new fence RFC.

Regards
P.

Re: [PATCH] drm: Don't BUG when a CRTC fence signals during a name lookup

[Christian König]

On 10/20/26 08:57, Tvrtko Ursulin wrote:
> 
> On 20/10/2026 07:53, Philipp Stanner wrote:
>> 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 <sushmit718@gmail.com>
>>
>> Reviewed-by: Philipp Stanner <phasta@kernel.org>
> 
> Hm this looked familiar so I looked around. There was an earlier fix for the same from Andre with msg-id 20260721-linux-drm_crtc_fix2-v3-2-afa8c71506e6@linaro.org which seems to have fallen through the cracks.

Yeah I was also wondering why that patch was never pushed.

I will take another look at this.

Regards,
Christian.

> 
> Regards,
> 
> Tvrtko
> 
>>
>>
>> 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
>