* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-04 1:50 [PATCH] drm/panthor: fix for dma-fence safe access rules Chia-I Wu
@ 2025-12-04 7:23 ` Boris Brezillon
2025-12-04 9:25 ` Liviu Dudau
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2025-12-04 7:23 UTC (permalink / raw)
To: Chia-I Wu
Cc: Steven Price, Liviu Dudau, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Grant Likely,
Heiko Stuebner, dri-devel, linux-kernel, tvrtko.ursulin
On Wed, 3 Dec 2025 17:50:34 -0800
Chia-I Wu <olvaffe@gmail.com> 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 <olvaffe@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 33b9ef537e359..a8b1347e4da71 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -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]);
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-04 1:50 [PATCH] drm/panthor: fix for dma-fence safe access rules Chia-I Wu
2025-12-04 7:23 ` Boris Brezillon
@ 2025-12-04 9:25 ` Liviu Dudau
2025-12-04 9:27 ` Tvrtko Ursulin
2025-12-04 16:56 ` Steven Price
3 siblings, 0 replies; 8+ messages in thread
From: Liviu Dudau @ 2025-12-04 9:25 UTC (permalink / raw)
To: Chia-I Wu
Cc: Boris Brezillon, Steven Price, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Grant Likely,
Heiko Stuebner, dri-devel, linux-kernel, tvrtko.ursulin
On Wed, Dec 03, 2025 at 05:50:34PM -0800, Chia-I Wu 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 <olvaffe@gmail.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Best regards,
Liviu
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 33b9ef537e359..a8b1347e4da71 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -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]);
>
> --
> 2.52.0.177.g9f829587af-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-04 1:50 [PATCH] drm/panthor: fix for dma-fence safe access rules Chia-I Wu
2025-12-04 7:23 ` Boris Brezillon
2025-12-04 9:25 ` Liviu Dudau
@ 2025-12-04 9:27 ` Tvrtko Ursulin
2025-12-04 17:42 ` Chia-I Wu
2025-12-04 16:56 ` Steven Price
3 siblings, 1 reply; 8+ messages in thread
From: Tvrtko Ursulin @ 2025-12-04 9:27 UTC (permalink / raw)
To: Chia-I Wu, Boris Brezillon, Steven Price, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Grant Likely, Heiko Stuebner, dri-devel,
linux-kernel
On 04/12/2025 01:50, Chia-I Wu 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 <olvaffe@gmail.com>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 33b9ef537e359..a8b1347e4da71 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -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]);
>
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-04 9:27 ` Tvrtko Ursulin
@ 2025-12-04 17:42 ` Chia-I Wu
2025-12-05 12:46 ` Boris Brezillon
0 siblings, 1 reply; 8+ messages in thread
From: Chia-I Wu @ 2025-12-04 17:42 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Grant Likely, Heiko Stuebner, dri-devel, linux-kernel
On Thu, Dec 4, 2025 at 1:27 AM Tvrtko Ursulin <tvrtko.ursulin@igalia.com> wrote:
>
>
> On 04/12/2025 01:50, Chia-I Wu 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 <olvaffe@gmail.com>
> > ---
> > drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> > index 33b9ef537e359..a8b1347e4da71 100644
> > --- a/drivers/gpu/drm/panthor/panthor_sched.c
> > +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> > @@ -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]);
> >
>
> 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.
>
> 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-04 17:42 ` Chia-I Wu
@ 2025-12-05 12:46 ` Boris Brezillon
2025-12-05 12:50 ` Tvrtko Ursulin
0 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2025-12-05 12:46 UTC (permalink / raw)
To: Chia-I Wu
Cc: Tvrtko Ursulin, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Grant Likely, Heiko Stuebner, dri-devel, linux-kernel
On Thu, 4 Dec 2025 09:42:37 -0800
Chia-I Wu <olvaffe@gmail.com> wrote:
> On Thu, Dec 4, 2025 at 1:27 AM Tvrtko Ursulin <tvrtko.ursulin@igalia.com> wrote:
> >
> >
> > On 04/12/2025 01:50, Chia-I Wu 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 <olvaffe@gmail.com>
> > > ---
> > > drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> > > index 33b9ef537e359..a8b1347e4da71 100644
> > > --- a/drivers/gpu/drm/panthor/panthor_sched.c
> > > +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> > > @@ -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]);
> > >
> >
> > 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;
> }
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.
>
> I thought it was "name" and added the "Fixes:" tag. But actually
> "sched" was also freed by group_release_work.
>
> >
> > 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
> >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-05 12:46 ` Boris Brezillon
@ 2025-12-05 12:50 ` Tvrtko Ursulin
0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2025-12-05 12:50 UTC (permalink / raw)
To: Boris Brezillon, Chia-I Wu
Cc: Steven Price, Liviu Dudau, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Grant Likely,
Heiko Stuebner, dri-devel, linux-kernel
On 05/12/2025 12:46, Boris Brezillon wrote:
> On Thu, 4 Dec 2025 09:42:37 -0800
> Chia-I Wu <olvaffe@gmail.com> wrote:
>
>> On Thu, Dec 4, 2025 at 1:27 AM Tvrtko Ursulin <tvrtko.ursulin@igalia.com> wrote:
>>>
>>>
>>> On 04/12/2025 01:50, Chia-I Wu 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 <olvaffe@gmail.com>
>>>> ---
>>>> drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
>>>> index 33b9ef537e359..a8b1347e4da71 100644
>>>> --- a/drivers/gpu/drm/panthor/panthor_sched.c
>>>> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
>>>> @@ -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]);
>>>>
>>>
>>> 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;
>> }
>
> 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
>
>>
>> I thought it was "name" and added the "Fixes:" tag. But actually
>> "sched" was also freed by group_release_work.
>>
>>>
>>> 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
>>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panthor: fix for dma-fence safe access rules
2025-12-04 1:50 [PATCH] drm/panthor: fix for dma-fence safe access rules Chia-I Wu
` (2 preceding siblings ...)
2025-12-04 9:27 ` Tvrtko Ursulin
@ 2025-12-04 16:56 ` Steven Price
3 siblings, 0 replies; 8+ messages in thread
From: Steven Price @ 2025-12-04 16:56 UTC (permalink / raw)
To: Chia-I Wu, Boris Brezillon, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Grant Likely, Heiko Stuebner, dri-devel, linux-kernel,
tvrtko.ursulin
On 04/12/2025 01:50, Chia-I Wu 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 <olvaffe@gmail.com>
Reviewed-by: Steven Price <steven.price@arm.com>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 33b9ef537e359..a8b1347e4da71 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -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]);
>
^ permalink raw reply [flat|nested] 8+ messages in thread