On Wed, 2026-07-08 at 16:22 +0100, André Draszik wrote:
> Since commit 541c8f2468b9 ("dma-buf: detach fence ops on signal v3"),
> I'm seeing the BUG_ON() triggering in drm_crtc's fence_to_crtc() via
> drm_crtc_fence_get_driver_name() regularly:
>
> Call trace:
> panic+0x58/0x5c
> die+0x160/0x178
> bug_brk_handler+0x70/0xa4
> call_el1_break_hook+0x3c/0x1a0
> do_el1_brk64+0x24/0x74
> el1_brk64+0x34/0x54
> el1h_64_sync_handler+0x80/0xfc
> el1h_64_sync+0x84/0x88
> drm_crtc_fence_get_driver_name+0x60/0x68 (P)
> sync_file_get_name+0x184/0x45c
> sync_file_ioctl+0x404/0xf70
> __arm64_sys_ioctl+0x124/0x1dc
>
> This looks to be caused by a code flow similar to the following:
>
> +++ snip +++
> thread A thread B
>
> ioctl(SYNC_IOC_FILE_INFO)
> sync_file_ioctl()
> sync_file_get_name()
> dma_fence_signal_timestamp_locked() dma_fence_driver_name()
> ops = rcu_dereference(fence->ops)
> if (!dma_fence_test_signaled_flag())
> ops->get_driver_name(fence) i.e.
> drm_crtc_fence_get_driver_name()
> test_and_set_bit(SIGNALED)
> RCU_INIT_POINTER(fence->ops, NULL)
> drm_crtc_fence_get_driver_name()
> BUG_ON(rcu_access_pointer(fence->ops)
> != &drm_crtc_fence_ops)
> +++ snap +++
>
> I see two ways to resolve this:
> a) simply drop the BUG_ON(). It can not work anymore since above
> commit, as it is racy now.
> b) pass the original 'ops' pointer obtained in dma_fence_driver_name()
> to all callees.
>
> This patch implements option a), as because:
> * I don't see much benefit in passing the extra pointer just for this
> BUG_ON() to work.
> * Requiring the dma_fence_ops in those callbacks is an implementation
> detail of the drm_crtc driver, and therefore upper layers shouldn't
> have to care about that.
> * The existence of the BUG_ON() doesn't appear to be consistent with
> implementations of ::get_driver_name() or ::get_timeline_name() in
> the majority of other DRM drivers in the first place. Those that do
> have a similar BUG_ON() (i915, xe) probably also need an update
> similar to this patch here but I'm not in a position to test those.
> * Using BUG() and friends to take down the system is an unacceptable
> way to handle a failure as evidenced by many threads on LKML and also
> in the kernel coding style. Here, the check was presumably added for
> detecting when something passes an invalid pointer, but that does not
> happen - and if it could, gracefully handling that situation would be
> more appropriate.
I think it was there to detect wrong fences.
I think that entire usage is effectively invalid since Christian's
rework.
>
> Note that the adjacent drm_crtc_fence_get_timeline_name() has the same
> problem and is fixed by this patch as well.
nit: It's cool to see folks write detailed commit messages, but I think
in this case it is very verbose. The presentation of the solution-
options IMO is more something for a comment in the mailing list ;)
The issue at hand is rather simple, AFAICS. Fences now set the ops
pointer to NULL on signal, which wasn't the case in the past, which
triggers the BUG_ON, which shouldn't be used anymore anyways. That's
the core of the issue.
The ops pointer never changed in the past. I suppose it was there to
check whether a wrong fence was passed to the container_of().
>
+Cc stable?
> Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
> Signed-off-by: André Draszik <[email protected]>
Reviewed-by: Philipp Stanner <[email protected]>
>
> ---
> v2:
> - don't turn fence_to_crtc() into macro
> - update commit message to include reference to unacceptable use of BUG
> ---
> drivers/gpu/drm/drm_crtc.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d55f1377ec36..36ae50ddf525 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -154,11 +154,8 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
> #endif
> }
>
> -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);
> return container_of(fence->extern_lock, struct drm_crtc, fence_lock);
> }
>
