Re: [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

On 21/07/2026 09:21, André Draszik wrote:
> 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.

Presumably these fence are exportable via syncobj and/or sync file? If 
so, your analysis seems correct to me. Fix as well looks it works to 
close the vulnerability, but the part I cannot help with, and which will 
need someone more familiar with the CRTC code and the associated object 
lifetimes, is to comment on whether a RCU delay in drm_crtc_cleanup() is 
acceptable or it must be made async one way or the other.

Regards,

Tvrtko

> Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
> 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.
>