* [PATCH] drm/panthor: Fix race when converting group handle to group object
@ 2024-09-23 10:34 Steven Price
2024-09-23 10:40 ` Boris Brezillon
2024-09-23 10:53 ` Liviu Dudau
0 siblings, 2 replies; 3+ messages in thread
From: Steven Price @ 2024-09-23 10:34 UTC (permalink / raw)
To: Boris Brezillon, Liviu Dudau; +Cc: Steven Price, dri-devel
XArray provides it's own internal lock which protects the internal array
when entries are being simultaneously added and removed. However there
is still a race between retrieving the pointer from the XArray and
incrementing the reference count.
To avoid this race simply hold the internal XArray lock when
incrementing the reference count, this ensures there cannot be a racing
call to xa_erase().
Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
Signed-off-by: Steven Price <steven.price@arm.com>
---
drivers/gpu/drm/panthor/panthor_sched.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 42afdf0ddb7e..0dbeebcf23b4 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -3242,6 +3242,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
return 0;
}
+static struct panthor_group *group_from_handle(struct panthor_group_pool *pool,
+ u32 group_handle)
+{
+ struct panthor_group *group;
+
+ xa_lock(&pool->xa);
+ group = group_get(xa_load(&pool->xa, group_handle));
+ xa_unlock(&pool->xa);
+
+ return group;
+}
+
int panthor_group_get_state(struct panthor_file *pfile,
struct drm_panthor_group_get_state *get_state)
{
@@ -3253,7 +3265,7 @@ int panthor_group_get_state(struct panthor_file *pfile,
if (get_state->pad)
return -EINVAL;
- group = group_get(xa_load(&gpool->xa, get_state->group_handle));
+ group = group_from_handle(gpool, get_state->group_handle);
if (!group)
return -EINVAL;
@@ -3384,7 +3396,7 @@ panthor_job_create(struct panthor_file *pfile,
job->call_info.latest_flush = qsubmit->latest_flush;
INIT_LIST_HEAD(&job->node);
- job->group = group_get(xa_load(&gpool->xa, group_handle));
+ job->group = group_from_handle(gpool, group_handle);
if (!job->group) {
ret = -EINVAL;
goto err_put_job;
--
2.39.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/panthor: Fix race when converting group handle to group object
2024-09-23 10:34 [PATCH] drm/panthor: Fix race when converting group handle to group object Steven Price
@ 2024-09-23 10:40 ` Boris Brezillon
2024-09-23 10:53 ` Liviu Dudau
1 sibling, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2024-09-23 10:40 UTC (permalink / raw)
To: Steven Price; +Cc: Liviu Dudau, dri-devel
On Mon, 23 Sep 2024 11:34:06 +0100
Steven Price <steven.price@arm.com> wrote:
> XArray provides it's own internal lock which protects the internal array
> when entries are being simultaneously added and removed. However there
> is still a race between retrieving the pointer from the XArray and
> incrementing the reference count.
>
> To avoid this race simply hold the internal XArray lock when
> incrementing the reference count, this ensures there cannot be a racing
> call to xa_erase().
>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Steven Price <steven.price@arm.com>
Uh, nice catch!
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 42afdf0ddb7e..0dbeebcf23b4 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3242,6 +3242,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
> return 0;
> }
>
> +static struct panthor_group *group_from_handle(struct panthor_group_pool *pool,
> + u32 group_handle)
> +{
> + struct panthor_group *group;
> +
> + xa_lock(&pool->xa);
> + group = group_get(xa_load(&pool->xa, group_handle));
> + xa_unlock(&pool->xa);
> +
> + return group;
> +}
> +
> int panthor_group_get_state(struct panthor_file *pfile,
> struct drm_panthor_group_get_state *get_state)
> {
> @@ -3253,7 +3265,7 @@ int panthor_group_get_state(struct panthor_file *pfile,
> if (get_state->pad)
> return -EINVAL;
>
> - group = group_get(xa_load(&gpool->xa, get_state->group_handle));
> + group = group_from_handle(gpool, get_state->group_handle);
> if (!group)
> return -EINVAL;
>
> @@ -3384,7 +3396,7 @@ panthor_job_create(struct panthor_file *pfile,
> job->call_info.latest_flush = qsubmit->latest_flush;
> INIT_LIST_HEAD(&job->node);
>
> - job->group = group_get(xa_load(&gpool->xa, group_handle));
> + job->group = group_from_handle(gpool, group_handle);
> if (!job->group) {
> ret = -EINVAL;
> goto err_put_job;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm/panthor: Fix race when converting group handle to group object
2024-09-23 10:34 [PATCH] drm/panthor: Fix race when converting group handle to group object Steven Price
2024-09-23 10:40 ` Boris Brezillon
@ 2024-09-23 10:53 ` Liviu Dudau
1 sibling, 0 replies; 3+ messages in thread
From: Liviu Dudau @ 2024-09-23 10:53 UTC (permalink / raw)
To: Steven Price; +Cc: Boris Brezillon, dri-devel
On Mon, Sep 23, 2024 at 11:34:06AM +0100, Steven Price wrote:
> XArray provides it's own internal lock which protects the internal array
> when entries are being simultaneously added and removed. However there
> is still a race between retrieving the pointer from the XArray and
> incrementing the reference count.
>
> To avoid this race simply hold the internal XArray lock when
> incrementing the reference count, this ensures there cannot be a racing
> call to xa_erase().
>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Steven Price <steven.price@arm.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Best regards,
Liviu
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 42afdf0ddb7e..0dbeebcf23b4 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3242,6 +3242,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
> return 0;
> }
>
> +static struct panthor_group *group_from_handle(struct panthor_group_pool *pool,
> + u32 group_handle)
> +{
> + struct panthor_group *group;
> +
> + xa_lock(&pool->xa);
> + group = group_get(xa_load(&pool->xa, group_handle));
> + xa_unlock(&pool->xa);
> +
> + return group;
> +}
> +
> int panthor_group_get_state(struct panthor_file *pfile,
> struct drm_panthor_group_get_state *get_state)
> {
> @@ -3253,7 +3265,7 @@ int panthor_group_get_state(struct panthor_file *pfile,
> if (get_state->pad)
> return -EINVAL;
>
> - group = group_get(xa_load(&gpool->xa, get_state->group_handle));
> + group = group_from_handle(gpool, get_state->group_handle);
> if (!group)
> return -EINVAL;
>
> @@ -3384,7 +3396,7 @@ panthor_job_create(struct panthor_file *pfile,
> job->call_info.latest_flush = qsubmit->latest_flush;
> INIT_LIST_HEAD(&job->node);
>
> - job->group = group_get(xa_load(&gpool->xa, group_handle));
> + job->group = group_from_handle(gpool, group_handle);
> if (!job->group) {
> ret = -EINVAL;
> goto err_put_job;
> --
> 2.39.5
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-23 10:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-23 10:34 [PATCH] drm/panthor: Fix race when converting group handle to group object Steven Price
2024-09-23 10:40 ` Boris Brezillon
2024-09-23 10:53 ` Liviu Dudau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.