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.
Note that the adjacent drm_crtc_fence_get_timeline_name() has the same
problem and is fixed by this patch as well.
Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
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);
}
--
2.55.0.795.g602f6c329a-goog
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 <andre.draszik@linaro.org>
Reviewed-by: Philipp Stanner <phasta@kernel.org>
>
> ---
> 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);
> }
>
© 2016 - 2026 Red Hat, Inc.