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

[Philipp Stanner <phasta@mailbox.org>]

On Tue, 2026-07-21 at 09:21 +0100, 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

nit: no one will be preempted here since the RCU read lock must be
held. The Sashiko tool misses the point, which is simply that someone
illegally frees up stuff that might be still in use, with or without
delay or preemption, that's all irrelevant for the issue.

Anyways, thanks for fixing this:

> 
> 

[…]

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

Reviewed-by: Philipp Stanner <phasta@kernel.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.