[PATCH v3 0/2] drm/drm_crtc: dma_fence_ops fixes

[PATCH v3 0/2] drm/drm_crtc: dma_fence_ops fixes

[André Draszik]

Hi,

These patches fix two issues in the drm/drm_crtc driver. Initially I
was hitting the BUG_ON() in a scenario as explained in the commit
message of what is now the second patch in this series.

After posting, sashiko.dev noticed another issue, that was previously
masked by the now-removed BUG_ON(). Since we can't have a loud BUG() be
replaced with silent data corruption or worse, I've also added a patch
to address this issue highlighted by sashiko.dev. I believe its
observation and analysis to be correct.

Cheers,
Andre'

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
Changes in v3:
- Philipp:
  - patch 1: update kerneldoc, add Fixes:
  - patch 2: shorten commit message
  - explicitly Cc: stable
  - collect tag
- Link to v2: https://lore.kernel.org/r/20260708-linux-drm_crtc_fix2-v2-0-cf72be75d75a@linaro.org

Changes in v2:
- add new patch 1 to address sashiko observation
- original patch 1 becomes patch 2
- patch 2:
  - don't turn fence_to_crtc() into macro (Jani, Philipp)
  - update commit message to include reference to deprecated use of BUG
- Link to v1: https://lore.kernel.org/r/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org

---
André Draszik (2):
      drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
      drm/drm_crtc: fix race with dma_fence_signal() in ::get_driver_name()

 drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
---
base-commit: 0718283ab28bc3907e10b61a6b4be6fefa1cbb2f
change-id: 20260618-linux-drm_crtc_fix2-23a7c354a412

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

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

[André Draszik]

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

-- 
2.55.0.229.g6434b31f56-goog

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

[Philipp Stanner]

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.

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

[Christian König]

On 7/21/26 13:20, Philipp Stanner wrote:
> 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:

Seconded.

> 
>>
>>
> 
> […]
> 
>> 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>

Reviewed-by: Christian König <christian.koenig@amd.com> for both patches.

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

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

[Tvrtko Ursulin]

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

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

[André Draszik]

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);
 }
 

-- 
2.55.0.229.g6434b31f56-goog

Re: [PATCH v3 0/2] drm/drm_crtc: dma_fence_ops fixes

[André Draszik]

On Tue, 2026-07-21 at 09:21 +0100, André Draszik wrote:
> Hi,
> 
> These patches fix two issues in the drm/drm_crtc driver. Initially I
> was hitting the BUG_ON() in a scenario as explained in the commit
> message of what is now the second patch in this series.
> 
> After posting, sashiko.dev noticed another issue, that was previously
> masked by the now-removed BUG_ON(). Since we can't have a loud BUG() be
> replaced with silent data corruption or worse, I've also added a patch
> to address this issue highlighted by sashiko.dev. I believe its
> observation and analysis to be correct.
> 
> Cheers,
> Andre'

Friendly ping on this series.

Cheers,
Andre'