Re: [PATCH] drm/drm_crtc: fix race with dma_fence_signal() in ::get_driver_name()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Use-After-Free in get_driver_name() and get_timeline_name() due to dma_fence RCU contract violation during CRTC teardown.
--

commit 4e03eb31730ff05ffb65d7d539f01df4c6b89099
Author: André Draszik <andre.draszik@linaro.org>

drm/drm_crtc: fix race with dma_fence_signal() in ::get_driver_name()

This patch removes a BUG_ON() in drm_crtc_fence_get_driver_name() that
was triggering because the dma_fence_ops get detached upon signaling.

> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 63ead8ba67564..31c8636e74677 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c

[ ... ]

> @@ -154,14 +157,6 @@ 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);
> -}
> -
>  static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
>  {
>  	struct drm_crtc *crtc = fence_to_crtc(fence);

[Severity: Critical]
This is a pre-existing issue, but does removing this BUG_ON() unmask a
use-after-free during device teardown?

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

While the BUG_ON() was causing spurious panics during normal operation,
it seems it was also inadvertently preventing this use-after-free from
proceeding silently.

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?

> 
> 	return crtc->dev->driver->name;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1