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

[Lazar, Lijo]

On 4/23/2024 7:13 AM, Srinivasan Shanmugam wrote:
> The buffer size is determined by the declaration char fw_name[30]; This
> means fw_name can hold up to 30 characters, including the null character
> that marks the end of the string.
> 
> The string to be written is "amdgpu/%s_mec.bin" or "amdgpu/%s_rlc.bin",
> where %s will be replaced by the value of chip_name.
> 
> The length of the string "amdgpu/%s_mec.bin" or "amdgpu/%s_rlc.bin"
> without the %s is 16 characters.
> 
> The warning message is saying that the chip_name could be up to 29
> characters long. If we add the 16 characters from the string
> "amdgpu/%s_mec.bin" or "amdgpu/%s_rlc.bin", we get a total of 45
> characters.
> 
> This is potentially longer than the buffer size of 30 characters.
> 
> if chip_name is longer than 14 characters (30 buffer size - 16
> characters from the string), the resulting string will not fit into the
> fw_name buffer, Thus increasing fw_name buffer size to 50
> 
> 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>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 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..1c46d5f6677f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> @@ -370,7 +370,7 @@ static void gfx_v9_4_3_free_microcode(struct amdgpu_device *adev)
>  static int gfx_v9_4_3_init_rlc_microcode(struct amdgpu_device *adev,
>  					  const char *chip_name)
>  {
> -	char fw_name[30];
> +	char fw_name[50];

Thanks for the patch. You may solve this by changing ucode_prefix to 15
in gfx_v9_4_3_init_microcode.

Thanks,
Lijo

>  	int err;
>  	const struct rlc_firmware_header_v2_0 *rlc_hdr;
>  	uint16_t version_major;
> @@ -407,7 +407,7 @@ static void gfx_v9_4_3_check_if_need_gfxoff(struct amdgpu_device *adev)
>  static int gfx_v9_4_3_init_cp_compute_microcode(struct amdgpu_device *adev,
>  					  const char *chip_name)
>  {
> -	char fw_name[30];
> +	char fw_name[50];
>  	int err;
>  
>  	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec.bin", chip_name);