[PATCH] drm/amdgpu: avoid double drm_exec_fini() in userq validate

[PATCH] drm/amdgpu: avoid double drm_exec_fini() in userq validate

[hongyan Xu]

From: Hongyan Xu <getshell@seu.edu.cn>

When new_addition is true, amdgpu_userq_vm_validate() calls
drm_exec_fini(&exec) before iterating over the collected HMM ranges and
calling amdgpu_ttm_tt_get_user_pages().

If amdgpu_ttm_tt_get_user_pages() fails in that path, the code jumps to
unlock_all and calls drm_exec_fini(&exec) a second time on the same
exec object. drm_exec_fini() is not idempotent: it frees exec->objects
and may also drop exec->contended and finalize the ww acquire context.

Route that error path directly to the range cleanup once exec has
already been finalized.

Fixes: 42f148788469 ("drm/amdgpu/userqueue: validate userptrs for userqueues")
Issue found using a prototype static analysis tool
and confirmed by code review.

Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 9d67b770bcc2..fe49108fabbd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1193,7 +1193,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
 			bo = range->bo;
 			ret = amdgpu_ttm_tt_get_user_pages(bo, range);
 			if (ret)
-				goto unlock_all;
+				goto free_ranges;
 		}
 
 		invalidated = true;
@@ -1220,6 +1220,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
 
 unlock_all:
 	drm_exec_fini(&exec);
+free_ranges:
 	xa_for_each(&xa, tmp_key, range) {
 		if (!range)
 			continue;
-- 
2.50.1.windows.1

Re: [PATCH] drm/amdgpu: avoid double drm_exec_fini() in userq validate

[hongyan Xu]

Please ignore the patch I sent previously. It was sent in error.

Sorry for the noise.

Thanks,
hongyan Xu

[PATCH] drm/amdgpu: avoid double drm_exec_fini() in userq validate

[Hongyan Xu]

When new_addition is true, amdgpu_userq_vm_validate() calls
drm_exec_fini(&exec) before iterating over the collected HMM ranges and
calling amdgpu_ttm_tt_get_user_pages().

If amdgpu_ttm_tt_get_user_pages() fails in that path, the code jumps to
unlock_all and calls drm_exec_fini(&exec) a second time on the same
exec object. drm_exec_fini() is not idempotent: it frees exec->objects
and may also drop exec->contended and finalize the ww acquire context.

Route that error path directly to the range cleanup once exec has
already been finalized.

Fixes: 42f148788469 ("drm/amdgpu/userqueue: validate userptrs for userqueues")
Issue found using a prototype static analysis tool
and confirmed by code review.

Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
Signed-off-by: Slavin Liu <220245772@seu.edu.cn>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 9d67b770bcc2..fe49108fabbd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1193,7 +1193,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
 			bo = range->bo;
 			ret = amdgpu_ttm_tt_get_user_pages(bo, range);
 			if (ret)
-				goto unlock_all;
+				goto free_ranges;
 		}
 
 		invalidated = true;
@@ -1220,6 +1220,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
 
 unlock_all:
 	drm_exec_fini(&exec);
+free_ranges:
 	xa_for_each(&xa, tmp_key, range) {
 		if (!range)
 			continue;
-- 
2.50.1.windows.1

Re: [PATCH] drm/amdgpu: avoid double drm_exec_fini() in userq validate

[Christian König]

On 4/22/26 14:38, Hongyan Xu wrote:
> When new_addition is true, amdgpu_userq_vm_validate() calls
> drm_exec_fini(&exec) before iterating over the collected HMM ranges and
> calling amdgpu_ttm_tt_get_user_pages().
> 
> If amdgpu_ttm_tt_get_user_pages() fails in that path, the code jumps to
> unlock_all and calls drm_exec_fini(&exec) a second time on the same
> exec object. drm_exec_fini() is not idempotent: it frees exec->objects
> and may also drop exec->contended and finalize the ww acquire context.
> 
> Route that error path directly to the range cleanup once exec has
> already been finalized.
> 
> Fixes: 42f148788469 ("drm/amdgpu/userqueue: validate userptrs for userqueues")
> Issue found using a prototype static analysis tool
> and confirmed by code review.
> 
> Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
> Signed-off-by: Slavin Liu <220245772@seu.edu.cn>

Good catch, Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 9d67b770bcc2..fe49108fabbd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1193,7 +1193,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
>                         bo = range->bo;
>                         ret = amdgpu_ttm_tt_get_user_pages(bo, range);
>                         if (ret)
> -                               goto unlock_all;
> +                               goto free_ranges;
>                 }
> 
>                 invalidated = true;
> @@ -1220,6 +1220,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
> 
>  unlock_all:
>         drm_exec_fini(&exec);
> +free_ranges:
>         xa_for_each(&xa, tmp_key, range) {
>                 if (!range)
>                         continue;
> --
> 2.50.1.windows.1
> 

Re: [PATCH] drm/amdgpu: avoid double drm_exec_fini() in userq validate

[Alex Deucher]

Applied.  Thanks!

On Wed, Apr 22, 2026 at 8:59 AM Christian König
<christian.koenig@amd.com> wrote:
>
> On 4/22/26 14:38, Hongyan Xu wrote:
> > When new_addition is true, amdgpu_userq_vm_validate() calls
> > drm_exec_fini(&exec) before iterating over the collected HMM ranges and
> > calling amdgpu_ttm_tt_get_user_pages().
> >
> > If amdgpu_ttm_tt_get_user_pages() fails in that path, the code jumps to
> > unlock_all and calls drm_exec_fini(&exec) a second time on the same
> > exec object. drm_exec_fini() is not idempotent: it frees exec->objects
> > and may also drop exec->contended and finalize the ww acquire context.
> >
> > Route that error path directly to the range cleanup once exec has
> > already been finalized.
> >
> > Fixes: 42f148788469 ("drm/amdgpu/userqueue: validate userptrs for userqueues")
> > Issue found using a prototype static analysis tool
> > and confirmed by code review.
> >
> > Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
> > Signed-off-by: Slavin Liu <220245772@seu.edu.cn>
>
> Good catch, Reviewed-by: Christian König <christian.koenig@amd.com>
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 9d67b770bcc2..fe49108fabbd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -1193,7 +1193,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
> >                         bo = range->bo;
> >                         ret = amdgpu_ttm_tt_get_user_pages(bo, range);
> >                         if (ret)
> > -                               goto unlock_all;
> > +                               goto free_ranges;
> >                 }
> >
> >                 invalidated = true;
> > @@ -1220,6 +1220,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
> >
> >  unlock_all:
> >         drm_exec_fini(&exec);
> > +free_ranges:
> >         xa_for_each(&xa, tmp_key, range) {
> >                 if (!range)
> >                         continue;
> > --
> > 2.50.1.windows.1
> >
>