* [PATCH] drm/amdgpu: Fix 'fw_name' buffer size to prevent truncations in amdgpu_mes_init_microcode
@ 2024-03-21 5:46 Srinivasan Shanmugam
2024-03-21 8:17 ` Lazar, Lijo
0 siblings, 1 reply; 2+ messages in thread
From: Srinivasan Shanmugam @ 2024-03-21 5:46 UTC (permalink / raw)
To: Christian König, Alex Deucher
Cc: amd-gfx, Srinivasan Shanmugam, Lijo Lazar
The snprintf function is used to write a formatted string into fw_name.
The format of the string is "amdgpu/%s_mes%s.bin", where %s is replaced
by the string in ucode_prefix and the second %s is replaced by either
"_2" or "1" depending on the condition pipe == AMDGPU_MES_SCHED_PIPE.
The length of the string "amdgpu/%s_mes%s.bin" is 16 characters plus the
length of ucode_prefix and the length of the string "_2" or "1". The
size of ucode_prefix is 30, so the maximum length of ucode_prefix is 29
characters (since one character is needed for the null terminator).
Therefore, the maximum possible length of the string written into
fw_name is 16 + 29 + 2 = 47 characters.
The size of fw_name is 40, so if the length of the string written into
fw_name is more than 39 characters (since one character is needed for
the null terminator), it will be truncated by the snprintf function, and
thus warnings will be seen.
By increasing the size of fw_name to 50, we ensure that fw_name is
large enough to hold the maximum possible length of the string, so the
snprintf function will not truncate the output.
Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c: In function ‘amdgpu_mes_init_microcode’:
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1482:66: warning: ‘%s’ directive output may be truncated writing up to 1 bytes into a region of size between 0 and 29 [-Wformat-truncation=]
1482 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
| ^~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1482:17: note: ‘snprintf’ output between 16 and 46 bytes into a destination of size 40
1482 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1483 | ucode_prefix,
| ~~~~~~~~~~~~~
1484 | pipe == AMDGPU_MES_SCHED_PIPE ? "" : "1");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:66: warning: ‘%s’ directive output may be truncated writing 1 byte into a region of size between 0 and 29 [-Wformat-truncation=]
1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
| ^~
1478 | ucode_prefix,
1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
| ~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:17: note: ‘snprintf’ output between 17 and 46 bytes into a destination of size 40
1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1478 | ucode_prefix,
| ~~~~~~~~~~~~~
1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:66: warning: ‘%s’ directive output may be truncated writing 2 bytes into a region of size between 0 and 29 [-Wformat-truncation=]
1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
| ^~
1478 | ucode_prefix,
1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
| ~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:17: note: ‘snprintf’ output between 18 and 47 bytes into a destination of size 40
1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1478 | ucode_prefix,
| ~~~~~~~~~~~~~
1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1489:62: warning: ‘_mes.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=]
1489 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes.bin",
| ^~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1489:17: note: ‘snprintf’ output between 16 and 45 bytes into a destination of size 40
1489 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes.bin",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1490 | ucode_prefix);
| ~~~~~~~~~~~~~
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Suggested-by: Lijo Lazar <lijo.lazar@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index bc8906403270..78dfd027dc99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -1467,7 +1467,7 @@ int amdgpu_mes_init_microcode(struct amdgpu_device *adev, int pipe)
const struct mes_firmware_header_v1_0 *mes_hdr;
struct amdgpu_firmware_info *info;
char ucode_prefix[30];
- char fw_name[40];
+ char fw_name[50];
bool need_retry = false;
int r;
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amdgpu: Fix 'fw_name' buffer size to prevent truncations in amdgpu_mes_init_microcode
2024-03-21 5:46 [PATCH] drm/amdgpu: Fix 'fw_name' buffer size to prevent truncations in amdgpu_mes_init_microcode Srinivasan Shanmugam
@ 2024-03-21 8:17 ` Lazar, Lijo
0 siblings, 0 replies; 2+ messages in thread
From: Lazar, Lijo @ 2024-03-21 8:17 UTC (permalink / raw)
To: Srinivasan Shanmugam, Christian König, Alex Deucher; +Cc: amd-gfx
On 3/21/2024 11:16 AM, Srinivasan Shanmugam wrote:
> The snprintf function is used to write a formatted string into fw_name.
> The format of the string is "amdgpu/%s_mes%s.bin", where %s is replaced
> by the string in ucode_prefix and the second %s is replaced by either
> "_2" or "1" depending on the condition pipe == AMDGPU_MES_SCHED_PIPE.
>
> The length of the string "amdgpu/%s_mes%s.bin" is 16 characters plus the
> length of ucode_prefix and the length of the string "_2" or "1". The
> size of ucode_prefix is 30, so the maximum length of ucode_prefix is 29
> characters (since one character is needed for the null terminator).
> Therefore, the maximum possible length of the string written into
> fw_name is 16 + 29 + 2 = 47 characters.
>
> The size of fw_name is 40, so if the length of the string written into
> fw_name is more than 39 characters (since one character is needed for
> the null terminator), it will be truncated by the snprintf function, and
> thus warnings will be seen.
>
> By increasing the size of fw_name to 50, we ensure that fw_name is
> large enough to hold the maximum possible length of the string, so the
> snprintf function will not truncate the output.
>
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c: In function ‘amdgpu_mes_init_microcode’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1482:66: warning: ‘%s’ directive output may be truncated writing up to 1 bytes into a region of size between 0 and 29 [-Wformat-truncation=]
> 1482 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
> | ^~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1482:17: note: ‘snprintf’ output between 16 and 46 bytes into a destination of size 40
> 1482 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1483 | ucode_prefix,
> | ~~~~~~~~~~~~~
> 1484 | pipe == AMDGPU_MES_SCHED_PIPE ? "" : "1");
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:66: warning: ‘%s’ directive output may be truncated writing 1 byte into a region of size between 0 and 29 [-Wformat-truncation=]
> 1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
> | ^~
> 1478 | ucode_prefix,
> 1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
> | ~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:17: note: ‘snprintf’ output between 17 and 46 bytes into a destination of size 40
> 1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1478 | ucode_prefix,
> | ~~~~~~~~~~~~~
> 1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:66: warning: ‘%s’ directive output may be truncated writing 2 bytes into a region of size between 0 and 29 [-Wformat-truncation=]
> 1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
> | ^~
> 1478 | ucode_prefix,
> 1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
> | ~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:17: note: ‘snprintf’ output between 18 and 47 bytes into a destination of size 40
> 1477 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes%s.bin",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1478 | ucode_prefix,
> | ~~~~~~~~~~~~~
> 1479 | pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1489:62: warning: ‘_mes.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=]
> 1489 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes.bin",
> | ^~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1489:17: note: ‘snprintf’ output between 16 and 45 bytes into a destination of size 40
> 1489 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes.bin",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1490 | ucode_prefix);
> | ~~~~~~~~~~~~~
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Suggested-by: Lijo Lazar <lijo.lazar@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Thanks,
Lijo
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index bc8906403270..78dfd027dc99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -1467,7 +1467,7 @@ int amdgpu_mes_init_microcode(struct amdgpu_device *adev, int pipe)
> const struct mes_firmware_header_v1_0 *mes_hdr;
> struct amdgpu_firmware_info *info;
> char ucode_prefix[30];
> - char fw_name[40];
> + char fw_name[50];
> bool need_retry = false;
> int r;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-03-21 8:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-21 5:46 [PATCH] drm/amdgpu: Fix 'fw_name' buffer size to prevent truncations in amdgpu_mes_init_microcode Srinivasan Shanmugam
2024-03-21 8:17 ` Lazar, Lijo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.