* [PATCH] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode
@ 2024-04-19 9:25 Srinivasan Shanmugam
2024-04-19 9:30 ` [PATCH v2] " Srinivasan Shanmugam
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Srinivasan Shanmugam @ 2024-04-19 9:25 UTC (permalink / raw
To: Christian König, Alex Deucher
Cc: amd-gfx, Srinivasan Shanmugam, Li Ma, Likun Gao, Lijo Lazar,
Kenneth Feng
This commit addresses buffer overflow in the smu_v14_0_init_microcode
function. The issue was about the snprintf function writing more bytes
into the fw_name buffer than it can hold.
The line of code is:
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
Here, snprintf is used to write a formatted string into fw_name. The
format is "amdgpu/%s.bin", where %s is a placeholder for the string
ucode_prefix. The sizeof(fw_name) argument tells snprintf the maximum
number of bytes it can write into fw_name, including the
null-terminating character. In the original code, fw_name is an array of
30 characters.
The string "amdgpu/%s.bin" could be up to 41 bytes long, which exceeds
the 30 bytes allocated for fw_name. This is because %s could be replaced
by ucode_prefix, which can be up to 29 characters long. Adding the 12
characters from "amdgpu/" and ".bin", the total length could be 41
characters.
To address this, the size of fw_name has been increased to 50
characters.
Fixes: fe6cd9152464 ("drm/amd/swsmu: add smu14 ip support")
Cc: Li Ma <li.ma@amd.com>
Cc: Likun Gao <Likun.Gao@amd.com>
Cc: Lijo Lazar <lijo.lazar@amd.com>
Cc: Kenneth Feng <kenneth.feng@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
index 7d2055b9d19f..5d9335cb8530 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
@@ -64,7 +64,7 @@ MODULE_FIRMWARE("amdgpu/smu_14_0_3.bin");
int smu_v14_0_init_microcode(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
- char fw_name[30];
+ char fw_name[50];
char ucode_prefix[30];
int err = 0;
const struct smc_firmware_header_v1_0 *hdr;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode
2024-04-19 9:25 [PATCH] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode Srinivasan Shanmugam
@ 2024-04-19 9:30 ` Srinivasan Shanmugam
2024-04-19 15:39 ` [PATCH v3] " Srinivasan Shanmugam
2024-04-19 15:44 ` [PATCH v4] " Srinivasan Shanmugam
2 siblings, 0 replies; 5+ messages in thread
From: Srinivasan Shanmugam @ 2024-04-19 9:30 UTC (permalink / raw
To: Christian König, Alex Deucher
Cc: amd-gfx, Srinivasan Shanmugam, Li Ma, Likun Gao, Lijo Lazar,
Kenneth Feng
This commit addresses buffer overflow in the smu_v14_0_init_microcode
function. The issue was about the snprintf function writing more bytes
into the fw_name buffer than it can hold.
The line of code is:
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
Here, snprintf is used to write a formatted string into fw_name. The
format is "amdgpu/%s.bin", where %s is a placeholder for the string
ucode_prefix. The sizeof(fw_name) argument tells snprintf the maximum
number of bytes it can write into fw_name, including the
null-terminating character. In the original code, fw_name is an array of
30 characters.
The string "amdgpu/%s.bin" could be up to 41 bytes long, which exceeds
the 30 bytes allocated for fw_name. This is because %s could be replaced
by ucode_prefix, which can be up to 29 characters long. Adding the 12
characters from "amdgpu/" and ".bin", the total length could be 41
characters.
To address this, the size of fw_name has been increased to 50
characters.
Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c: In function ‘smu_v14_0_init_microcode’:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=]
80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
| ^~ ~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:9: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 30
80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: fe6cd9152464 ("drm/amd/swsmu: add smu14 ip support")
Cc: Li Ma <li.ma@amd.com>
Cc: Likun Gao <Likun.Gao@amd.com>
Cc: Lijo Lazar <lijo.lazar@amd.com>
Cc: Kenneth Feng <kenneth.feng@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v2:
- Updated commit message
drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
index 7d2055b9d19f..5d9335cb8530 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
@@ -64,7 +64,7 @@ MODULE_FIRMWARE("amdgpu/smu_14_0_3.bin");
int smu_v14_0_init_microcode(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
- char fw_name[30];
+ char fw_name[50];
char ucode_prefix[30];
int err = 0;
const struct smc_firmware_header_v1_0 *hdr;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode
2024-04-19 9:25 [PATCH] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode Srinivasan Shanmugam
2024-04-19 9:30 ` [PATCH v2] " Srinivasan Shanmugam
@ 2024-04-19 15:39 ` Srinivasan Shanmugam
2024-04-19 15:44 ` [PATCH v4] " Srinivasan Shanmugam
2 siblings, 0 replies; 5+ messages in thread
From: Srinivasan Shanmugam @ 2024-04-19 15:39 UTC (permalink / raw
To: Christian König, Alex Deucher
Cc: amd-gfx, Srinivasan Shanmugam, Li Ma, Likun Gao, Lijo Lazar,
Kenneth Feng
This commit addresses buffer overflow in the smu_v14_0_init_microcode
function. The issue was about the snprintf function writing more bytes
into the fw_name buffer than it can hold.
The line of code is:
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
Here, snprintf is used to write a formatted string into fw_name. The
format is "amdgpu/%s.bin", where %s is a placeholder for the string
ucode_prefix. The sizeof(fw_name) argument tells snprintf the maximum
number of bytes it can write into fw_name, including the
null-terminating character. In the original code, fw_name is an array of
30 characters.
The string "amdgpu/%s.bin" could be up to 41 bytes long, which exceeds
the 30 bytes allocated for fw_name. This is because %s could be replaced
by ucode_prefix, which can be up to 29 characters long. Adding the 12
characters from "amdgpu/" and ".bin", the total length could be 41
characters.
To address this, the size of ucode_prefix has been reduced to 15
characters. This ensures that the maximum length of the string written
into fw_name does not exceed its capacity.
As smu_13/14 etc. don't follow legacy scheme ie.,
amdgpu_ucode_legacy_naming
Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c: In function ‘smu_v14_0_init_microcode’:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=]
80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
| ^~ ~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:9: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 30
80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: fe6cd9152464 ("drm/amd/swsmu: add smu14 ip support")
Cc: Li Ma <li.ma@amd.com>
Cc: Likun Gao <Likun.Gao@amd.com>
Cc: Lijo Lazar <lijo.lazar@amd.com>
Cc: Kenneth Feng <kenneth.feng@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Suggested-by: Lijo Lazar <lijo.lazar@amd.com>
---
v3:
- Reduced ucode_prefix to 15 instead of fw_name size increasing as
smu_13/14 etc. don't follow legacy scheme ie.,
amdgpu_ucode_legacy_naming (Lijo)
drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
index 7d2055b9d19f..5d9335cb8530 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
@@ -64,7 +64,7 @@ MODULE_FIRMWARE("amdgpu/smu_14_0_3.bin");
int smu_v14_0_init_microcode(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
- char fw_name[30];
+ char fw_name[50];
char ucode_prefix[30];
int err = 0;
const struct smc_firmware_header_v1_0 *hdr;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v4] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode
2024-04-19 9:25 [PATCH] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode Srinivasan Shanmugam
2024-04-19 9:30 ` [PATCH v2] " Srinivasan Shanmugam
2024-04-19 15:39 ` [PATCH v3] " Srinivasan Shanmugam
@ 2024-04-19 15:44 ` Srinivasan Shanmugam
2024-04-22 13:01 ` Lazar, Lijo
2 siblings, 1 reply; 5+ messages in thread
From: Srinivasan Shanmugam @ 2024-04-19 15:44 UTC (permalink / raw
To: Christian König, Alex Deucher
Cc: amd-gfx, Srinivasan Shanmugam, Li Ma, Likun Gao, Lijo Lazar,
Kenneth Feng
This commit addresses buffer overflow in the smu_v14_0_init_microcode
function. The issue was about the snprintf function writing more bytes
into the fw_name buffer than it can hold.
The line of code is:
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
Here, snprintf is used to write a formatted string into fw_name. The
format is "amdgpu/%s.bin", where %s is a placeholder for the string
ucode_prefix. The sizeof(fw_name) argument tells snprintf the maximum
number of bytes it can write into fw_name, including the
null-terminating character. In the original code, fw_name is an array of
30 characters.
The string "amdgpu/%s.bin" could be up to 41 bytes long, which exceeds
the 30 bytes allocated for fw_name. This is because %s could be replaced
by ucode_prefix, which can be up to 29 characters long. Adding the 12
characters from "amdgpu/" and ".bin", the total length could be 41
characters.
To address this, the size of ucode_prefix has been reduced to 15
characters. This ensures that the maximum length of the string written
into fw_name does not exceed its capacity.
As smu_13/14 etc. don't follow legacy scheme ie.,
amdgpu_ucode_legacy_naming
Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c: In function ‘smu_v14_0_init_microcode’:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=]
80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
| ^~ ~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:9: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 30
80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: fe6cd9152464 ("drm/amd/swsmu: add smu14 ip support")
Cc: Li Ma <li.ma@amd.com>
Cc: Likun Gao <Likun.Gao@amd.com>
Cc: Lijo Lazar <lijo.lazar@amd.com>
Cc: Kenneth Feng <kenneth.feng@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v4:
- Reduced ucode_prefix to 15 instead of fw_name size increasing as
smu_13/14 etc. don't follow legacy scheme ie.,
amdgpu_ucode_legacy_naming (Lijo)
drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
index 7d2055b9d19f..68b9bf822e8d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
@@ -65,7 +65,7 @@ int smu_v14_0_init_microcode(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
char fw_name[30];
- char ucode_prefix[30];
+ char ucode_prefix[15];
int err = 0;
const struct smc_firmware_header_v1_0 *hdr;
const struct common_firmware_header *header;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode
2024-04-19 15:44 ` [PATCH v4] " Srinivasan Shanmugam
@ 2024-04-22 13:01 ` Lazar, Lijo
0 siblings, 0 replies; 5+ messages in thread
From: Lazar, Lijo @ 2024-04-22 13:01 UTC (permalink / raw
To: Srinivasan Shanmugam, Christian König, Alex Deucher
Cc: amd-gfx, Li Ma, Likun Gao, Kenneth Feng
On 4/19/2024 9:14 PM, Srinivasan Shanmugam wrote:
> This commit addresses buffer overflow in the smu_v14_0_init_microcode
> function. The issue was about the snprintf function writing more bytes
> into the fw_name buffer than it can hold.
>
> The line of code is:
>
> snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>
> Here, snprintf is used to write a formatted string into fw_name. The
> format is "amdgpu/%s.bin", where %s is a placeholder for the string
> ucode_prefix. The sizeof(fw_name) argument tells snprintf the maximum
> number of bytes it can write into fw_name, including the
> null-terminating character. In the original code, fw_name is an array of
> 30 characters.
>
> The string "amdgpu/%s.bin" could be up to 41 bytes long, which exceeds
> the 30 bytes allocated for fw_name. This is because %s could be replaced
> by ucode_prefix, which can be up to 29 characters long. Adding the 12
> characters from "amdgpu/" and ".bin", the total length could be 41
> characters.
>
> To address this, the size of ucode_prefix has been reduced to 15
> characters. This ensures that the maximum length of the string written
> into fw_name does not exceed its capacity.
>
> As smu_13/14 etc. don't follow legacy scheme ie.,
> amdgpu_ucode_legacy_naming
>
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c: In function ‘smu_v14_0_init_microcode’:
> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=]
> 80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
> | ^~ ~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu14/smu_v14_0.c:80:9: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 30
> 80 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: fe6cd9152464 ("drm/amd/swsmu: add smu14 ip support")
> Cc: Li Ma <li.ma@amd.com>
> Cc: Likun Gao <Likun.Gao@amd.com>
> Cc: Lijo Lazar <lijo.lazar@amd.com>
> Cc: Kenneth Feng <kenneth.feng@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Thanks,
Lijo
> ---
> v4:
> - Reduced ucode_prefix to 15 instead of fw_name size increasing as
> smu_13/14 etc. don't follow legacy scheme ie.,
> amdgpu_ucode_legacy_naming (Lijo)
>
> drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
> index 7d2055b9d19f..68b9bf822e8d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
> @@ -65,7 +65,7 @@ int smu_v14_0_init_microcode(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> char fw_name[30];
> - char ucode_prefix[30];
> + char ucode_prefix[15];
> int err = 0;
> const struct smc_firmware_header_v1_0 *hdr;
> const struct common_firmware_header *header;
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-22 13:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 9:25 [PATCH] drm/amdgpu: Fix snprintf buffer size in smu_v14_0_init_microcode Srinivasan Shanmugam
2024-04-19 9:30 ` [PATCH v2] " Srinivasan Shanmugam
2024-04-19 15:39 ` [PATCH v3] " Srinivasan Shanmugam
2024-04-19 15:44 ` [PATCH v4] " Srinivasan Shanmugam
2024-04-22 13:01 ` 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.