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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Incomplete fix: UAF due to dangling dma_fence->extern_lock and RCU-001 violation
--

--- Patch [1]: [PATCH v2 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind ---
commit be7b3e18f378ca6990d9039f7a24dee91cbb6d7d
Author: André Draszik <andre.draszik@linaro.org>

drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind

This commit attempts to fix a use-after-free when userspace accesses a
dma_fence whose underlying CRTC has been destroyed. It adds a
synchronize_rcu() call to drm_crtc_cleanup() to ensure dma_fence_ops
remain valid during an RCU grace period.

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

> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -501,6 +501,12 @@ 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();

[Severity: Critical]
This is a pre-existing issue, but the fix seems incomplete and might leave
critical use-after-free paths open.

First, does this sequence leave the fence's external lock dangling? When
drm_crtc_create_fence() initializes the fence, it uses &crtc->fence_lock
as the external lock:

drm_crtc_create_fence() {
    ...
    dma_fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
                   crtc->fence_context, ++crtc->fence_seqno);
    ...
}

If userspace calls ioctl(SYNC_IOC_FILE_INFO), sync_fill_fence_info()
unconditionally calls dma_fence_get_status(fence). This calls
dma_fence_lock_irqsave(), which attempts to acquire fence->lock.

If the CRTC has been freed, wouldn't this result in a use-after-free when
acquiring the spinlock? The synchronize_rcu() here protects RCU readers,
but fence->lock is accessed outside of RCU.

Second, does this violate the RCU removal-before-reclaim ordering if the
fence was never signaled?

If the fence isn't signaled (for example, during an error or unplug path),
dma_fence_signal() is never called and fence->ops is never set to NULL.

If fence->ops is not cleared before synchronize_rcu() is called, could new
RCU readers start after the grace period ends, see a non-NULL fence->ops,
and access the freed CRTC memory via ops->get_driver_name(fence)?

> +
>  	/* Note that the crtc_list is considered to be static; should we
>  	 * remove the drm_crtc at runtime we would have to decrement all

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-linux-drm_crtc_fix2-v2-0-cf72be75d75a@linaro.org?part=1