[PATCH] drm/panthor: Fix TOCTOU race between GROUP_REGISTERED check and erase

[PATCH] drm/panthor: Fix TOCTOU race between GROUP_REGISTERED check and erase

[Osama Abdelkader]

panthor_group_destroy() checks the GROUP_REGISTERED mark and then calls
xa_erase() without holding the xarray lock across both. If the handle is
destroyed by another thread in between, and a concurrent GROUP_CREATE
reuses the freed ID, the first destroy erases and releases the new,
still-initializing group, leading to a use-after-free in
panthor_group_create().

Do the mark check and erase atomically under xa_lock().

Fixes: eec7e23d848d ("drm/panthor: Prevent potential UAF in group creation")
Cc: stable@vger.kernel.org
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/gpu/drm/panthor/panthor_sched.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 0493858e55d7..a343c5180e01 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -3770,12 +3770,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
 	struct panthor_group_pool *gpool = pfile->groups;
 	struct panthor_device *ptdev = pfile->ptdev;
 	struct panthor_scheduler *sched = ptdev->scheduler;
-	struct panthor_group *group;
+	struct panthor_group *group = NULL;
 
-	if (!xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
-		return -EINVAL;
+	/*
+	 * Check the mark and erase the entry atomically, so a concurrent
+	 * destroy + create can't make us erase a group that's still being
+	 * initialized and happens to reuse the same handle.
+	 */
+	xa_lock(&gpool->xa);
+	if (xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
+		group = __xa_erase(&gpool->xa, group_handle);
+	xa_unlock(&gpool->xa);
 
-	group = xa_erase(&gpool->xa, group_handle);
 	if (!group)
 		return -EINVAL;
 
-- 
2.53.0

Re: [PATCH] drm/panthor: Fix TOCTOU race between GROUP_REGISTERED check and erase

[Boris Brezillon]

On Wed,  7 Oct 2026 20:12:19 +0200
Osama Abdelkader <osama.abdelkader@gmail.com> wrote:

> panthor_group_destroy() checks the GROUP_REGISTERED mark and then calls
> xa_erase() without holding the xarray lock across both. If the handle is
> destroyed by another thread in between, and a concurrent GROUP_CREATE
> reuses the freed ID, the first destroy erases and releases the new,
> still-initializing group, leading to a use-after-free in
> panthor_group_create().
> 
> Do the mark check and erase atomically under xa_lock().
> 
> Fixes: eec7e23d848d ("drm/panthor: Prevent potential UAF in group creation")

The fix looks legit, but I wonder how the bug was found. If this was
found with the help of a tool (AI or static code analyzer), this should
be reflected with an `Assisted-by` tag (see [1]).

With the relevant Assisted-by tag, this is

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

[1]https://docs.kernel.org/process/coding-assistants.html

> Cc: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0493858e55d7..a343c5180e01 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3770,12 +3770,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
>  	struct panthor_group_pool *gpool = pfile->groups;
>  	struct panthor_device *ptdev = pfile->ptdev;
>  	struct panthor_scheduler *sched = ptdev->scheduler;
> -	struct panthor_group *group;
> +	struct panthor_group *group = NULL;
>  
> -	if (!xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
> -		return -EINVAL;
> +	/*
> +	 * Check the mark and erase the entry atomically, so a concurrent
> +	 * destroy + create can't make us erase a group that's still being
> +	 * initialized and happens to reuse the same handle.
> +	 */
> +	xa_lock(&gpool->xa);
> +	if (xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
> +		group = __xa_erase(&gpool->xa, group_handle);
> +	xa_unlock(&gpool->xa);
>  
> -	group = xa_erase(&gpool->xa, group_handle);
>  	if (!group)
>  		return -EINVAL;
>  

Re: [PATCH] drm/panthor: Fix TOCTOU race between GROUP_REGISTERED check and erase

[Liviu Dudau]

On Wed, Oct 07, 2026 at 08:12:19PM +0200, Osama Abdelkader wrote:
> panthor_group_destroy() checks the GROUP_REGISTERED mark and then calls
> xa_erase() without holding the xarray lock across both. If the handle is
> destroyed by another thread in between, and a concurrent GROUP_CREATE
> reuses the freed ID, the first destroy erases and releases the new,
> still-initializing group, leading to a use-after-free in
> panthor_group_create().
> 
> Do the mark check and erase atomically under xa_lock().
> 
> Fixes: eec7e23d848d ("drm/panthor: Prevent potential UAF in group creation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

As Boris mentioned, please let us know if LLM has assisted in finding the bug or in
the creation of the patch before we can merge the fix.

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0493858e55d7..a343c5180e01 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3770,12 +3770,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
>  	struct panthor_group_pool *gpool = pfile->groups;
>  	struct panthor_device *ptdev = pfile->ptdev;
>  	struct panthor_scheduler *sched = ptdev->scheduler;
> -	struct panthor_group *group;
> +	struct panthor_group *group = NULL;
>  
> -	if (!xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
> -		return -EINVAL;
> +	/*
> +	 * Check the mark and erase the entry atomically, so a concurrent
> +	 * destroy + create can't make us erase a group that's still being
> +	 * initialized and happens to reuse the same handle.
> +	 */
> +	xa_lock(&gpool->xa);
> +	if (xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
> +		group = __xa_erase(&gpool->xa, group_handle);
> +	xa_unlock(&gpool->xa);
>  
> -	group = xa_erase(&gpool->xa, group_handle);
>  	if (!group)
>  		return -EINVAL;
>  
> -- 
> 2.53.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] drm/panthor: Fix TOCTOU race between GROUP_REGISTERED check and erase

[Osama Abdelkader]

On Thu, Oct 08, 2026 at 09:53:50AM +0100, Liviu Dudau wrote:
> On Wed, Oct 07, 2026 at 08:12:19PM +0200, Osama Abdelkader wrote:
> > panthor_group_destroy() checks the GROUP_REGISTERED mark and then calls
> > xa_erase() without holding the xarray lock across both. If the handle is
> > destroyed by another thread in between, and a concurrent GROUP_CREATE
> > reuses the freed ID, the first destroy erases and releases the new,
> > still-initializing group, leading to a use-after-free in
> > panthor_group_create().
> > 
> > Do the mark check and erase atomically under xa_lock().
> > 
> > Fixes: eec7e23d848d ("drm/panthor: Prevent potential UAF in group creation")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> As Boris mentioned, please let us know if LLM has assisted in finding the bug or in
> the creation of the patch before we can merge the fix.
> 

Hi Liviu and Boris,

Thanks for the review, an LLM has assisted to find the bug but I created the fix patch myself.
I think I have to add the tag nevertheless.

Thanks,
Osama

> Best regards,
> Liviu
> 
> > ---
> >  drivers/gpu/drm/panthor/panthor_sched.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> > index 0493858e55d7..a343c5180e01 100644
> > --- a/drivers/gpu/drm/panthor/panthor_sched.c
> > +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> > @@ -3770,12 +3770,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
> >  	struct panthor_group_pool *gpool = pfile->groups;
> >  	struct panthor_device *ptdev = pfile->ptdev;
> >  	struct panthor_scheduler *sched = ptdev->scheduler;
> > -	struct panthor_group *group;
> > +	struct panthor_group *group = NULL;
> >  
> > -	if (!xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
> > -		return -EINVAL;
> > +	/*
> > +	 * Check the mark and erase the entry atomically, so a concurrent
> > +	 * destroy + create can't make us erase a group that's still being
> > +	 * initialized and happens to reuse the same handle.
> > +	 */
> > +	xa_lock(&gpool->xa);
> > +	if (xa_get_mark(&gpool->xa, group_handle, GROUP_REGISTERED))
> > +		group = __xa_erase(&gpool->xa, group_handle);
> > +	xa_unlock(&gpool->xa);
> >  
> > -	group = xa_erase(&gpool->xa, group_handle);
> >  	if (!group)
> >  		return -EINVAL;
> >  
> > -- 
> > 2.53.0
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯