drm/panthor: fix for dma-fence safe access rules

PATCH Superseded Chia-I Wu 5 participants 8 messages Dec 4, 2025 – Dec 5, 2025

Subsystems (3)

DRM drivers DRM misc Arm Mali panthor DRM driver

lore

Share & tools

lore

Chia-I Wu

author

2025-12-04 01:50 UTC #0 lore raw

PATCH 1/1 drm/panthor: fix for dma-fence safe access rules

Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.

Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")

Signed-off-by Chia-I Wu <[email protected]>

---
 drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/gpu/drm/panthor/panthor_sched.c +4 -0

index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
 #include "panthor_devfreq.h"
 #include "panthor_device.h"

@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
 						   release_work);
 	u32 i;
 
+	/* dma-fences may still be accessing group->queues under rcu lock. */
+	synchronize_rcu();
+
 	for (i = 0; i < group->queue_count; i++)
 		group_free_queue(group, group->queues[i]);

Signature · 1 line

2.52.0.177.g9f829587af-goog

Boris Brezillon

maintainer Reviewed-by on v1 1/1

2025-12-04 07:24 UTC #1 lore raw

Quoted 7 lines · On Wed, 3 Dec 2025 17:50:34 -0800 Chia-I Wu <[email protected]> wrote:

Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>

Reviewed-by Boris Brezillon <[email protected]>

Quoted 26 lines

---
 drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/gpu/drm/panthor/panthor_sched.c +4 -0

index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
 #include "panthor_devfreq.h"
 #include "panthor_device.h"

@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
 						   release_work);
 	u32 i;
 
+	/* dma-fences may still be accessing group->queues under rcu lock. */
+	synchronize_rcu();
+
 	for (i = 0; i < group->queue_count; i++)
 		group_free_queue(group, group->queues[i]);

Liviu Dudau

maintainer Reviewed-by on v1 1/1

2025-12-04 09:25 UTC #2 lore raw

In reply to Chia-I Wu

Quoted 7 lines · Chia-I Wu

Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>

Reviewed-by Liviu Dudau <[email protected]>

Best regards,
Liviu

Quoted 29 lines

---
 drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/gpu/drm/panthor/panthor_sched.c +4 -0

index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
 #include "panthor_devfreq.h"
 #include "panthor_device.h"

@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
 						   release_work);
 	u32 i;
 
+	/* dma-fences may still be accessing group->queues under rcu lock. */
+	synchronize_rcu();
+
 	for (i = 0; i < group->queue_count; i++)
 		group_free_queue(group, group->queues[i]);

Signature · 1 line

2.52.0.177.g9f829587af-goog

Tvrtko Ursulin

2025-12-04 09:27 UTC #3 lore raw

In reply to Chia-I Wu

Quoted 24 lines · Chia-I Wu

Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>
---
  drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
  1 file changed, 4 insertions(+)
drivers/gpu/drm/panthor/panthor_sched.c +1 -0
index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
  #include <linux/module.h>
  #include <linux/platform_device.h>
  #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
  #include "panthor_devfreq.h"
  #include "panthor_device.h"

drivers/gpu/drm/panthor/panthor_sched.c static void group_release_work(struct work_struct *work) · line 932

Quoted 9 lines

drivers/gpu/drm/panthor/panthor_sched.c +3 -0

@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
  						   release_work);
  	u32 i;
 
+	/* dma-fences may still be accessing group->queues under rcu lock. */
+	synchronize_rcu();
+
  	for (i = 0; i < group->queue_count; i++)
  		group_free_queue(group, group->queues[i]);

This handles the shared queue->fence_ctx.lock as well (which is also unsafe until Christian lands the inline lock, etc patch series) so it looks good to me as well.

Just to mention an alternative could be to simply switch release_work to INIT_RCU_WORK/queue_rcu_work, but I am not sure if that has an advantage.

Regards,

Tvrtko

Chia-I Wu

author

2025-12-04 17:42 UTC #4 lore raw

Quoted 30 lines · Tvrtko Ursulin

Quoted 27 lines · Chia-I Wu
Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>
---
  drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
  1 file changed, 4 insertions(+)
drivers/gpu/drm/panthor/panthor_sched.c +1 -0
index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
  #include <linux/module.h>
  #include <linux/platform_device.h>
  #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
  #include "panthor_devfreq.h"
  #include "panthor_device.h"
@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
                                                 release_work);
      u32 i;
 

Quoted 10 lines

drivers/gpu/drm/panthor/panthor_sched.c static void group_release_work(struct work_struct *work) · line 932
Quoted 6 lines
drivers/gpu/drm/panthor/panthor_sched.c +3 -0
+     /* dma-fences may still be accessing group->queues under rcu lock. */
+     synchronize_rcu();
+
      for (i = 0; i < group->queue_count; i++)
              group_free_queue(group, group->queues[i]);
 
This handles the shared queue->fence_ctx.lock as well (which is also unsafe until Christian lands the inline lock, etc patch series) so it looks good to me as well.

Yeah, I will send v2 to drop the misleading "Fixes:" tag.

FWIW, the UAF I saw was from accessing the string returned by

static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)

{
        struct drm_sched_fence *fence = to_drm_sched_fence(f);
        return (const char *)fence->sched->name;
}

I thought it was "name" and added the "Fixes:" tag. But actually "sched" was also freed by group_release_work.

Quoted 8 lines

Just to mention an alternative could be to simply switch release_work to INIT_RCU_WORK/queue_rcu_work, but I am not sure if that has an advantage.
Regards,
Tvrtko

Boris Brezillon

maintainer

2025-12-05 12:46 UTC #5 lore raw

Quoted 43 lines · On Thu, 4 Dec 2025 09:42:37 -0800 Chia-I Wu <[email protected]> wrote:

Quoted 30 lines · Tvrtko Ursulin
Quoted 27 lines · Chia-I Wu
Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>
---
  drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
  1 file changed, 4 insertions(+)
drivers/gpu/drm/panthor/panthor_sched.c +1 -0
index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
  #include <linux/module.h>
  #include <linux/platform_device.h>
  #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
  #include "panthor_devfreq.h"
  #include "panthor_device.h"
@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
                                                 release_work);
      u32 i;
 
Quoted 10 lines
drivers/gpu/drm/panthor/panthor_sched.c static void group_release_work(struct work_struct *work) · line 932
Quoted 6 lines
drivers/gpu/drm/panthor/panthor_sched.c +3 -0
+     /* dma-fences may still be accessing group->queues under rcu lock. */
+     synchronize_rcu();
+
      for (i = 0; i < group->queue_count; i++)
              group_free_queue(group, group->queues[i]);
 
This handles the shared queue->fence_ctx.lock as well (which is also unsafe until Christian lands the inline lock, etc patch series) so it looks good to me as well.
Yeah, I will send v2 to drop the misleading "Fixes:" tag.

Quoted 7 lines

FWIW, the UAF I saw was from accessing the string returned by
static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
{
        struct drm_sched_fence *fence = to_drm_sched_fence(f);
        return (const char *)fence->sched->name;
}

IIRC, the only place calling this callback is some debugsfs knob dumping fences attached to dma_buf resvs, and we're not supposed to expose our driver fences to the outside world (we use the drm_sched_fence proxy for that), so I'm curious where the access was coming from.

Quoted 12 lines

I thought it was "name" and added the "Fixes:" tag. But actually "sched" was also freed by group_release_work.
Quoted 8 lines
Just to mention an alternative could be to simply switch release_work to INIT_RCU_WORK/queue_rcu_work, but I am not sure if that has an advantage.
Regards,
Tvrtko

Tvrtko Ursulin

2025-12-05 12:50 UTC #6 lore raw

Quoted 54 lines · Boris Brezillon

Quoted 43 lines · On Thu, 4 Dec 2025 09:42:37 -0800 Chia-I Wu <[email protected]> wrote:
Quoted 30 lines · Tvrtko Ursulin
Quoted 27 lines · Chia-I Wu
Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>
---
   drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
   1 file changed, 4 insertions(+)
drivers/gpu/drm/panthor/panthor_sched.c +1 -0
index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
   #include <linux/module.h>
   #include <linux/platform_device.h>
   #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
   #include "panthor_devfreq.h"
   #include "panthor_device.h"
@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
                                                  release_work);
       u32 i;
 
Quoted 10 lines
drivers/gpu/drm/panthor/panthor_sched.c static void group_release_work(struct work_struct *work) · line 932
Quoted 6 lines
drivers/gpu/drm/panthor/panthor_sched.c +3 -0
+     /* dma-fences may still be accessing group->queues under rcu lock. */
+     synchronize_rcu();
+
       for (i = 0; i < group->queue_count; i++)
               group_free_queue(group, group->queues[i]);
 
This handles the shared queue->fence_ctx.lock as well (which is also unsafe until Christian lands the inline lock, etc patch series) so it looks good to me as well.
Yeah, I will send v2 to drop the misleading "Fixes:" tag.
Quoted 7 lines
FWIW, the UAF I saw was from accessing the string returned by
static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
{
         struct drm_sched_fence *fence = to_drm_sched_fence(f);
         return (const char *)fence->sched->name;
}

Quoted 5 lines

IIRC, the only place calling this callback is some debugsfs knob dumping fences attached to dma_buf resvs, and we're not supposed to expose our driver fences to the outside world (we use the drm_sched_fence proxy for that), so I'm curious where the access was coming from.

Via the sync_file uapi.

For reference here is a reproducer for xe:
https://patchwork.freedesktop.org/patch/642709/?series=146211&rev=2

Regards,

Tvrtko

Quoted 14 lines

Quoted 13 lines
I thought it was "name" and added the "Fixes:" tag. But actually "sched" was also freed by group_release_work.
Quoted 8 lines
Just to mention an alternative could be to simply switch release_work to INIT_RCU_WORK/queue_rcu_work, but I am not sure if that has an advantage.
Regards,
Tvrtko

Steven Price

maintainer Reviewed-by on v1 1/1

2025-12-04 16:57 UTC #7 lore raw

In reply to Chia-I Wu

Quoted 7 lines · Chia-I Wu

Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document the rules") details the dma-fence safe access rules. The most common culprit is that drm_sched_fence_get_timeline_name may race with group_free_queue.
Fixes d2624d90a0b77 ("drm/panthor: assign unique names to queues")
Signed-off-by Chia-I Wu <[email protected]>

Reviewed-by Steven Price <[email protected]>

Quoted 26 lines

---
 drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/gpu/drm/panthor/panthor_sched.c +4 -0

index 33b9ef537e359..a8b1347e4da71 100644
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/rcupdate.h>
 
 #include "panthor_devfreq.h"
 #include "panthor_device.h"

@@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work)
 						   release_work);
 	u32 i;
 
+	/* dma-fences may still be accessing group->queues under rcu lock. */
+	synchronize_rcu();
+
 	for (i = 0; i < group->queue_count; i++)
 		group_free_queue(group, group->queues[i]);