In [1], sashiko reported the following issue:

=== snip === Looking at how these fences are managed, drm_crtc_create_fence() creates a dma_fence without taking a reference to the drm_device or drm_crtc. Because the sync_file framework exposes this fence to userspace, the fence can outlive the CRTC.

The dma_fence contract requires that data accessed by dma_fence_ops (like get_driver_name) must remain valid for an RCU grace period after the fence is signaled. However, drm_crtc_cleanup() and the subsequent freeing of the device do not wait for an RCU grace period via synchronize_rcu().

If userspace calls ioctl(SYNC_IOC_FILE_INFO) concurrently with a device hot-unplug:

CPU1 (Userspace) sync_file_get_name() ops = rcu_dereference(fence->ops); if (!dma_fence_test_signaled_flag()) // Preempted or delayed here

CPU2 (Driver Teardown) Signals the fence (setting fence->ops = NULL) Destroys and frees the CRTC without waiting for an RCU grace period

CPU1 (Resumes) ops->get_driver_name(fence) -> drm_crtc_fence_get_driver_name() crtc = fence_to_crtc(fence); // Casts to the freed CRTC return crtc->dev->driver->name; // Use-after-free

...

Does the CRTC or DRM device need to be kept alive for the RCU grace period, or should the fence hold a proper reference to prevent the use-after-free when get_driver_name() and get_timeline_name() access the freed CRTC structure? === snap ===

I believe this to be a correct observation and this patch implements the suggestion of waiting for an RCU grace period before proceeding with destruction of the drm_crtc, so that get_driver_name() and get_timeline_name() can still work.

Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f3... Fixes: 6d6003c4b613 ("drm/fence: add fence timeline to drm_crtc") Cc: stable@vger.kernel.org Signed-off-by: André Draszik andre.draszik@linaro.org

--- v3: - Philipp: update kerneldoc, add Fixes:

v2: new patch --- drivers/gpu/drm/drm_crtc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 63ead8ba6756..e8e80c936852 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -493,14 +493,23 @@ EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes); * drm_crtc_cleanup - Clean up the core crtc usage * @crtc: CRTC to cleanup * - * This function cleans up @crtc and removes it from the DRM mode setting - * core. Note that the function does *not* free the crtc structure itself, - * this is the responsibility of the caller. + * This function cleans up @crtc and removes it from the DRM mode setting core, + * after first waiting an RCU grace period to ensure @crtc->dev can safely be + * dereferenced by our dma_fence_ops. + * + * Note that the function does *not* free the crtc structure itself, this is the + * responsibility of the caller. */ void drm_crtc_cleanup(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev;

+ /* Ensure our dma_fence_ops remain valid for an RCU grace period after + * the fence is signaled. This is necessary because our dma_fence_ops + * dereference crtc->dev. + */ + synchronize_rcu(); + /* Note that the crtc_list is considered to be static; should we * remove the drm_crtc at runtime we would have to decrement all * the indices on the drm_crtc after us in the crtc_list.

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

In other words, a fence's ops pointer can now change to NULL on signal, which wasn't the case before the culprit commit and hence the BUG_ON() triggers.

Simply drop the BUG_ON() as it's usage of fence ops is invalid now, and because using BUG_ON() and friends to take down the system is an unacceptable way to handle a failure.

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") Cc: stable@vger.kernel.org Reviewed-by: Philipp Stanner phasta@kernel.org Signed-off-by: André Draszik andre.draszik@linaro.org --- v3: - Philipp: - shorten commit message - explicitly Cc: stable - collect tag

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 e8e80c936852..5295ec122e0d 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); }