[PATCH v2] drm/amdgpu: Fix buffer size in gfx_v9_4_3_init_ cp_compute_microcode() and rlc_microcode()

[Lazar, Lijo]

On 4/25/2024 12:05 PM, Srinivasan Shanmugam wrote:
> The function gfx_v9_4_3_init_microcode in gfx_v9_4_3.c was generating
> about potential truncation of output when using the snprintf function.
> The issue was due to the size of the buffer 'ucode_prefix' being too
> small to accommodate the maximum possible length of the string being
> written into it.
> 
> The string being written is "amdgpu/%s_mec.bin" or "amdgpu/%s_rlc.bin",
> where %s is replaced by the value of 'chip_name'. The length of this
> string without the %s is 16 characters. The warning message indicated
> that 'chip_name' could be up to 29 characters long, resulting in a total
> of 45 characters, which exceeds the buffer size of 30 characters.
> 
> To resolve this issue, the size of the 'ucode_prefix' buffer has been
> reduced from 30 to 15. This ensures that the maximum possible length of
> the string being written into the buffer will not exceed its size, thus
> preventing potential buffer overflow and truncation issues.
> 
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c: In function ‘gfx_v9_4_3_early_init’:
> drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c:379:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=]
>   379 |         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", chip_name);
>       |                                                    ^~
> ......
>   439 |         r = gfx_v9_4_3_init_rlc_microcode(adev, ucode_prefix);
>       |                                                 ~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c:379:9: note: ‘snprintf’ output between 16 and 45 bytes into a destination of size 30
>   379 |         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_rlc.bin", chip_name);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c:413:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=]
>   413 |         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec.bin", chip_name);
>       |                                                    ^~
> ......
>   443 |         r = gfx_v9_4_3_init_cp_compute_microcode(adev, ucode_prefix);
>       |                                                        ~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c:413:9: note: ‘snprintf’ output between 16 and 45 bytes into a destination of size 30
>   413 |         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec.bin", chip_name);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: 86301129698b ("drm/amdgpu: split gc v9_4_3 functionality from gc v9_0")
> Cc: Hawking Zhang <Hawking.Zhang at amd.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Lijo Lazar <lijo.lazar at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> Suggested-by: Lijo Lazar <lijo.lazar at amd.com>

Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>

Thanks,
Lijo
> ---
> v2:
>  - reduced the size in ucode_prefix to 15 instead of changing size in
>    fw_name (Lijo)
> 
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> index 0e429b7ed036..7b16e8cca86a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> @@ -431,7 +431,7 @@ static int gfx_v9_4_3_init_cp_compute_microcode(struct amdgpu_device *adev,
>  
>  static int gfx_v9_4_3_init_microcode(struct amdgpu_device *adev)
>  {
> -	char ucode_prefix[30];
> +	char ucode_prefix[15];
>  	int r;
>  
>  	amdgpu_ucode_ip_version_decode(adev, GC_HWIP, ucode_prefix, sizeof(ucode_prefix));