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@xxxxxxx>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Lijo Lazar <lijo.lazar@xxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> Suggested-by: Lijo Lazar <lijo.lazar@xxxxxxx>
Reviewed-by: Lijo Lazar <lijo.lazar@xxxxxxx>
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));
