[PATCH 6.1] drm/amdgpu: Use scnprintf() in amdgpu_mes_add_ring()

[PATCH 6.1] drm/amdgpu: Use scnprintf() in amdgpu_mes_add_ring()

[Evgenii Burenchev]

Replace sprintf() with scnprintf() to prevent a potential buffer overflow
when writing to ring->name. The buffer size is 16 bytes. For compute rings,
the string format "compute_%d.%d.%d" can exceed this limit when the total
number of digits in the three numbers is greater than 5 (e.g., pasid=1234,
gang_id=0, queue_id=0). This can lead to memory corruption.

Using scnprintf() guarantees that the buffer is not overflowed, even if the
string is truncated. This is a minimal fix for the issue; the BUG() for
unknown queue types is left unchanged to avoid additional risk.

This code is only present in LTS kernels v6.12, v6.6, and v6.1, as it was
completely refactored in upstream. Therefore, this patch is specifically
intended for stable trees.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: d0c423b64765 ("drm/amdgpu/mes: use ring for kernel queue submission")
Signed-off-by: Evgenii Burenchev <evg28bur@yandex.ru>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 3feb792c210d..6208967f0e6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -1057,13 +1057,14 @@ int amdgpu_mes_add_ring(struct amdgpu_device *adev, int gang_id,
 	ring->doorbell_index = qprops.doorbell_off;
 
 	if (queue_type == AMDGPU_RING_TYPE_GFX)
-		sprintf(ring->name, "gfx_%d.%d.%d", pasid, gang_id, queue_id);
+		scnprintf(ring->name, sizeof(ring->name), "gfx_%d.%d.%d",
+			pasid, gang_id, queue_id);
 	else if (queue_type == AMDGPU_RING_TYPE_COMPUTE)
-		sprintf(ring->name, "compute_%d.%d.%d", pasid, gang_id,
-			queue_id);
+		scnprintf(ring->name, sizeof(ring->name), "compute_%d.%d.%d",
+			pasid, gang_id, queue_id);
 	else if (queue_type == AMDGPU_RING_TYPE_SDMA)
-		sprintf(ring->name, "sdma_%d.%d.%d", pasid, gang_id,
-			queue_id);
+		scnprintf(ring->name, sizeof(ring->name), "sdma_%d.%d.%d",
+			pasid, gang_id, queue_id);
 	else
 		BUG();
 
-- 
2.43.0

Re: [PATCH 6.1] drm/amdgpu: Use scnprintf() in amdgpu_mes_add_ring()

[sashiko-bot]

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

Pre-existing issues:
- [High] Double unlock of `adev->mes` mutex in error path.
- [High] Unchecked NULL fence pointers passed to `dma_fence_wait()`.
--

--- Patch [1]: [PATCH 6.1] drm/amdgpu: Use scnprintf() in amdgpu_mes_add_ring() ---
commit a3f6095725ba7df26135d6b5a8b649631f54d76e
Author: Evgenii Burenchev <evg28bur@yandex.ru>

drm/amdgpu: Use scnprintf() in amdgpu_mes_add_ring()

Replace sprintf() with scnprintf() to prevent a potential buffer overflow
when writing to ring->name.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 0c546245793b9..41e699f4b5d2e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -1053,13 +1053,14 @@ int amdgpu_mes_add_ring(struct amdgpu_device *adev, int gang_id,
>  	ring->doorbell_index = qprops.doorbell_off;
>  
>  	if (queue_type == AMDGPU_RING_TYPE_GFX)

[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer
dereference just before this code in amdgpu_mes_add_ring():

	dma_fence_wait(gang->process->vm->last_update, false);
	dma_fence_wait(ctx_data->meta_data_va->last_pt_update, false);

If the fences are unallocated, these pointers might be NULL. The
dma_fence_wait() function does not check for NULL inputs before dereferencing.
Could this result in a kernel oops during normal process/gang queue creation?

[Severity: High]
This is a pre-existing issue, but I also noticed a potential double unlock
of the adev->mes mutex. Before the new scnprintf() code, we unlock it here:

	amdgpu_mes_unlock(&adev->mes);

	r = amdgpu_mes_add_hw_queue(adev, gang_id, &qprops, &queue_id);
	if (r)
		goto clean_up_ring;

If amdgpu_mes_add_hw_queue() fails, it jumps to clean_up_ring, which falls
through to clean_up_memory:

clean_up_memory:
	kfree(ring);
	amdgpu_mes_unlock(&adev->mes);
	return r;

Will this second call to amdgpu_mes_unlock() cause a double unlock panic or
state corruption since the mutex was already dropped?

> -		sprintf(ring->name, "gfx_%d.%d.%d", pasid, gang_id, queue_id);
> +		scnprintf(ring->name, sizeof(ring->name), "gfx_%d.%d.%d",
> +			pasid, gang_id, queue_id);
>  	else if (queue_type == AMDGPU_RING_TYPE_COMPUTE)
> -		sprintf(ring->name, "compute_%d.%d.%d", pasid, gang_id,
> -			queue_id);
> +		scnprintf(ring->name, sizeof(ring->name), "compute_%d.%d.%d",
> +			pasid, gang_id, queue_id);
>  	else if (queue_type == AMDGPU_RING_TYPE_SDMA)
> -		sprintf(ring->name, "sdma_%d.%d.%d", pasid, gang_id,
> -			queue_id);
> +		scnprintf(ring->name, sizeof(ring->name), "sdma_%d.%d.%d",
> +			pasid, gang_id, queue_id);
>  	else
>  		BUG();
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709112825.40016-1-evg28bur@yandex.ru?part=1

Re: [PATCH 6.1] drm/amdgpu: Use scnprintf() in amdgpu_mes_add_ring()

[Christian König]

On 7/9/26 13:28, Evgenii Burenchev wrote:
> Replace sprintf() with scnprintf() to prevent a potential buffer overflow
> when writing to ring->name. The buffer size is 16 bytes. For compute rings,
> the string format "compute_%d.%d.%d" can exceed this limit when the total
> number of digits in the three numbers is greater than 5 (e.g., pasid=1234,
> gang_id=0, queue_id=0). This can lead to memory corruption.
> 
> Using scnprintf() guarantees that the buffer is not overflowed, even if the
> string is truncated. This is a minimal fix for the issue; the BUG() for
> unknown queue types is left unchanged to avoid additional risk.
> 
> This code is only present in LTS kernels v6.12, v6.6, and v6.1, as it was
> completely refactored in upstream. Therefore, this patch is specifically
> intended for stable trees.

That is not stuff which should ever go into a stable kernel.

This is just a minor cleanup and not a bug fix at all.

Regards,
Christian.

> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: d0c423b64765 ("drm/amdgpu/mes: use ring for kernel queue submission")
> Signed-off-by: Evgenii Burenchev <evg28bur@yandex.ru>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 3feb792c210d..6208967f0e6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -1057,13 +1057,14 @@ int amdgpu_mes_add_ring(struct amdgpu_device *adev, int gang_id,
>  	ring->doorbell_index = qprops.doorbell_off;
>  
>  	if (queue_type == AMDGPU_RING_TYPE_GFX)
> -		sprintf(ring->name, "gfx_%d.%d.%d", pasid, gang_id, queue_id);
> +		scnprintf(ring->name, sizeof(ring->name), "gfx_%d.%d.%d",
> +			pasid, gang_id, queue_id);
>  	else if (queue_type == AMDGPU_RING_TYPE_COMPUTE)
> -		sprintf(ring->name, "compute_%d.%d.%d", pasid, gang_id,
> -			queue_id);
> +		scnprintf(ring->name, sizeof(ring->name), "compute_%d.%d.%d",
> +			pasid, gang_id, queue_id);
>  	else if (queue_type == AMDGPU_RING_TYPE_SDMA)
> -		sprintf(ring->name, "sdma_%d.%d.%d", pasid, gang_id,
> -			queue_id);
> +		scnprintf(ring->name, sizeof(ring->name), "sdma_%d.%d.%d",
> +			pasid, gang_id, queue_id);
>  	else
>  		BUG();
>