[PATCH] drm/amdgpu: fix fence reference leak in amdgpu_gfx_run_cleaner_shader_job

[PATCH] drm/amdgpu: fix fence reference leak in amdgpu_gfx_run_cleaner_shader_job

[Wentao Liang]

In amdgpu_gfx_run_cleaner_shader_job(), amdgpu_job_submit() returns a
dma_fence with an elevated reference count. The function correctly
releases this reference on the success path after dma_fence_wait().
However, if dma_fence_wait() fails (e.g., due to a signal interruption),
the code jumps to the error label without calling dma_fence_put(),
resulting in a reference leak.

Fix the leak by adding dma_fence_put(f) before the goto err when
dma_fence_wait() returns an error.

Fixes: 559a285816af ("drm/amdgpu: Replace 'amdgpu_job_submit_direct' with 'drm_sched_entity' in cleaner shader")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index b8ca876694ff..88bec4e93712 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -1686,8 +1686,10 @@ static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
 	f = amdgpu_job_submit(job);
 
 	r = dma_fence_wait(f, false);
-	if (r)
+	if (r) {
+		dma_fence_put(f);
 		goto err;
+	}
 
 	dma_fence_put(f);
 
-- 
2.34.1

Re: [PATCH] drm/amdgpu: fix fence reference leak in amdgpu_gfx_run_cleaner_shader_job

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The indirect buffer (IB) allocation size is hardcoded to 64 bytes, but the initialization loop writes up to 1024 bytes based on the ring's alignment mask, leading to a severe out-of-bounds write.
- [High] The error handling branch for `dma_fence_wait` is dead code because uninterruptible waits with infinite timeouts do not return errors, resulting in silently ignored hardware failures.
--

commit 154fe15a3810311348d132047ef742f38daab7b7
Author: Wentao Liang <vulab@iscas.ac.cn>
drm/amdgpu: fix fence reference leak in amdgpu_gfx_run_cleaner_shader_job

This commit addresses a potential fence reference leak in the error path of
amdgpu_gfx_run_cleaner_shader_job(). It adds a dma_fence_put() before jumping
to the error label when dma_fence_wait() fails.

>     However, if dma_fence_wait() fails (e.g., due to a signal interruption),
>     the code jumps to the error label without calling dma_fence_put(),
>     resulting in a reference leak.

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for
dma_fence_wait() to fail due to a signal interruption here?

Since dma_fence_wait(f, false) calls dma_fence_wait_timeout() with
intr = false and an infinite timeout, it seems it cannot be interrupted by
signals or timeout, and will always return 0.

[Severity: Critical]
This is a pre-existing issue, but can the initialization loop write past the
end of the allocation?

Looking earlier in amdgpu_gfx_run_cleaner_shader_job():

drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:amdgpu_gfx_run_cleaner_shader_job() {
	...
	r = amdgpu_job_alloc_with_ib(ring->adev, &entity, owner,
				     64, 0, &job,
				     AMDGPU_KERNEL_JOB_ID_CLEANER_SHADER);
	...
	ib = &job->ibs[0];
	for (i = 0; i <= ring->funcs->align_mask; ++i)
		ib->ptr[i] = ring->funcs->nop;
	...
}

The indirect buffer is allocated with a hardcoded size of 64 bytes, but the
loop writes up to ring->funcs->align_mask + 1 dwords.

If the alignment mask is 0xff (255) for modern GFX compute rings, this would
write 256 dwords (1024 bytes), which exceeds the 64 byte allocation and may
corrupt subsequent blocks in the memory pool.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index ff5a55f5f3c9e..2784306400a81 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1729,8 +1729,10 @@ static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
>  	f = amdgpu_job_submit(job);
>  
>  	r = dma_fence_wait(f, false);
> -	if (r)
> +	if (r) {
> +		dma_fence_put(f);
>  		goto err;
> +	}
        ^^^^

[Severity: High]
This is a pre-existing issue, but does this make the error handling branch
dead code?

If dma_fence_wait() always returns 0, the newly added dma_fence_put() would
be unreachable.

Additionally, if the cleaner shader job hangs and the fence completes with an
error, would the failure be silently ignored since f->error is not checked?

>  
>  	dma_fence_put(f);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605091803.6018-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] drm/amdgpu: fix fence reference leak in amdgpu_gfx_run_cleaner_shader_job

[Alex Deucher]

On Fri, Jun 5, 2026 at 5:24 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> In amdgpu_gfx_run_cleaner_shader_job(), amdgpu_job_submit() returns a
> dma_fence with an elevated reference count. The function correctly
> releases this reference on the success path after dma_fence_wait().
> However, if dma_fence_wait() fails (e.g., due to a signal interruption),
> the code jumps to the error label without calling dma_fence_put(),
> resulting in a reference leak.
>
> Fix the leak by adding dma_fence_put(f) before the goto err when
> dma_fence_wait() returns an error.
>
> Fixes: 559a285816af ("drm/amdgpu: Replace 'amdgpu_job_submit_direct' with 'drm_sched_entity' in cleaner shader")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index b8ca876694ff..88bec4e93712 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1686,8 +1686,10 @@ static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
>         f = amdgpu_job_submit(job);
>
>         r = dma_fence_wait(f, false);
> -       if (r)
> +       if (r) {
> +               dma_fence_put(f);
>                 goto err;
> +       }

I think all of the clean up paths have issues.  How about something like this:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 321d7aa52f042..848846ac9391e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -1701,7 +1701,7 @@ static int
amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
                                  &sched, 1, NULL);
        if (r) {
                dev_err(adev->dev, "Failed setting up GFX kernel entity.\n");
-               goto err;
+               return r;
        }

        /*
@@ -1729,16 +1729,12 @@ static int
amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
        f = amdgpu_job_submit(job);

        r = dma_fence_wait(f, false);
-       if (r)
-               goto err;

        dma_fence_put(f);

+err:
        /* Clean up the scheduler entity */
        drm_sched_entity_destroy(&entity);
-       return 0;
-
-err:
        return r;
 }



>
>         dma_fence_put(f);
>
> --
> 2.34.1
>