[PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[Srinivasan Shanmugam]

The issue was present in the lines where 'fw_name' was being formatted.
This fix ensures that the output is not truncated

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                                                                  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                                                                  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive output may be truncated writing 4 bytes into a region of size between 2 and 31 [-Wformat-truncation=]
  105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
      |                                                                         ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 14 and 43 bytes into a destination of size 40
  105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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_vcn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9c514a606a2f..f994ee6c548d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work);
 int amdgpu_vcn_early_init(struct amdgpu_device *adev)
 {
 	char ucode_prefix[30];
-	char fw_name[40];
+	char fw_name[42];
 	int r, i;
 
 	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
-- 
2.34.1

Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[Christian König]

Am 20.03.24 um 09:45 schrieb Srinivasan Shanmugam:
> The issue was present in the lines where 'fw_name' was being formatted.
> This fix ensures that the output is not truncated
>
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>        |                                                                  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>        |                                                                  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive output may be truncated writing 4 bytes into a region of size between 2 and 31 [-Wformat-truncation=]
>    105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>        |                                                                         ^~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 14 and 43 bytes into a destination of size 40
>    105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> 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>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 9c514a606a2f..f994ee6c548d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work);
>   int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>   {
>   	char ucode_prefix[30];
> -	char fw_name[40];
> +	char fw_name[42];
>   	int r, i;
>   
>   	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {

Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[Lazar, Lijo]

On 3/20/2024 2:15 PM, Srinivasan Shanmugam wrote:
> The issue was present in the lines where 'fw_name' was being formatted.
> This fix ensures that the output is not truncated
> 
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                                                                  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                                                                  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive output may be truncated writing 4 bytes into a region of size between 2 and 31 [-Wformat-truncation=]
>   105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>       |                                                                         ^~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 14 and 43 bytes into a destination of size 40
>   105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> 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_vcn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 9c514a606a2f..f994ee6c548d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work);
>  int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>  {
>  	char ucode_prefix[30];

Hi Srini,

Sorry, if I miscommunicated. Suggestion was to reduce prefix size to 25
as the max prefix length is possibly length of dimgrey_cavefish_vcn.

With fw_name[42] also, you may run into 43 bytes (30 prefix + 13 for
others) warning.

Thanks,
Lijo

> -	char fw_name[40];
> +	char fw_name[42];
>  	int r, i;
>  
>  	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {

Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[SRINIVASAN SHANMUGAM]

On 3/20/2024 3:12 PM, Lazar, Lijo wrote:
>
> On 3/20/2024 2:15 PM, Srinivasan Shanmugam wrote:
>> The issue was present in the lines where 'fw_name' was being formatted.
>> This fix ensures that the output is not truncated
>>
>> Fixes the below with gcc W=1:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>>        |                                                                  ^
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>>        |                                                                  ^
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>>    102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive output may be truncated writing 4 bytes into a region of size between 2 and 31 [-Wformat-truncation=]
>>    105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>>        |                                                                         ^~~~
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 14 and 43 bytes into a destination of size 40
>>    105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> 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_vcn.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index 9c514a606a2f..f994ee6c548d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work);
>>   int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>>   {
>>   	char ucode_prefix[30];
> Hi Srini,
>
> Sorry, if I miscommunicated. Suggestion was to reduce prefix size to 25
> as the max prefix length is possibly length of dimgrey_cavefish_vcn.
Hi Lijo,

my mistake, the fw_name size must have been 53.

How 53? -> 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). The maximum number of digits in an integer is 10. 
Therefore, the maximum possible length of the string written into 
fw_name is 14 + 29 + 10 = 53 characters.

On the other hand reducing ucode_prefix to 25 from 30:

1. The length of the string "amdgpu/%s.bin" is 12 characters plus the 
length of ucode_prefix. The length of the string "amdgpu/%s_%d.bin" is 
14 characters plus the length of ucode_prefix and the number of digits in i.

If ucode_prefix is 25 characters long, the maximum length of the string 
written into fw_name is 14 + 25 + 1 (for a single digit i) = 40 
characters, which is exactly the size of fw_name.

Is that this solution assumes that i will not be more than 9 (a single 
digit)?. If i can be a number with more than one digit, should we need 
to increase the size of fw_name accordingly.?

2. If you reduce the size of ucode_prefix to 25, it means that it can 
store a version string of up to 24 characters (since one character is 
needed for the null terminator).

For example: if tomorrow, if we get something like 
dimgrey_cavefish_smc_xxxyyyzz - then in this case yyyzz would be lost? 
is that in "amdgpu_ucode_ip_version_decode " & 
"amdgpu_ucode_legacy_naming" , is that are we always ensuring that it 
will never be longer than 24 characters?

Thanks,
Srini
>
> With fw_name[42] also, you may run into 43 bytes (30 prefix + 13 for
> others) warning.
>
> Thanks,
> Lijo
>
>> -	char fw_name[40];
>> +	char fw_name[42];
>>   	int r, i;
>>   
>>   	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {

Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[Lazar, Lijo]

On 3/20/2024 8:28 PM, SRINIVASAN SHANMUGAM wrote:
> 
> On 3/20/2024 3:12 PM, Lazar, Lijo wrote:
>>
>> On 3/20/2024 2:15 PM, Srinivasan Shanmugam wrote:
>>> The issue was present in the lines where 'fw_name' was being formatted.
>>> This fix ensures that the output is not truncated
>>>
>>> Fixes the below with gcc W=1:
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function
>>> ‘amdgpu_vcn_early_init’:
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’
>>> output may be truncated before the last format character
>>> [-Wformat-truncation=]
>>>    102 |                 snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>       
>>> |                                                                  ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’
>>> output between 12 and 41 bytes into a destination of size 40
>>>    102 |                 snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>        |                
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’
>>> output may be truncated before the last format character
>>> [-Wformat-truncation=]
>>>    102 |                 snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>       
>>> |                                                                  ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’
>>> output between 12 and 41 bytes into a destination of size 40
>>>    102 |                 snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>        |                
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’
>>> directive output may be truncated writing 4 bytes into a region of
>>> size between 2 and 31 [-Wformat-truncation=]
>>>    105 |                         snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s_%d.bin", ucode_prefix, i);
>>>       
>>> |                                                                         ^~~~
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’
>>> output between 14 and 43 bytes into a destination of size 40
>>>    105 |                         snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s_%d.bin", ucode_prefix, i);
>>>        |                        
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> 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_vcn.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> index 9c514a606a2f..f994ee6c548d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> @@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct
>>> work_struct *work);
>>>   int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>>>   {
>>>       char ucode_prefix[30];
>> Hi Srini,
>>
>> Sorry, if I miscommunicated. Suggestion was to reduce prefix size to 25
>> as the max prefix length is possibly length of dimgrey_cavefish_vcn.
> Hi Lijo,
> 
> my mistake, the fw_name size must have been 53.
> 
> How 53? -> 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). The maximum number of digits in an integer is 10.
> Therefore, the maximum possible length of the string written into
> fw_name is 14 + 29 + 10 = 53 characters.
> 
> On the other hand reducing ucode_prefix to 25 from 30:
> 
> 1. The length of the string "amdgpu/%s.bin" is 12 characters plus the
> length of ucode_prefix. The length of the string "amdgpu/%s_%d.bin" is
> 14 characters plus the length of ucode_prefix and the number of digits
> in i.
> 

> If ucode_prefix is 25 characters long, the maximum length of the string
> written into fw_name is 14 + 25 + 1 (for a single digit i) = 40
> characters, which is exactly the size of fw_name.
> 
> Is that this solution assumes that i will not be more than 9 (a single
> digit)?. If i can be a number with more than one digit, should we need
> to increase the size of fw_name accordingly.?
> 

With 25, fw_name size required is 24 (no null char) + 13 (rest of string
including null char) + 1 (digit). 'i' is assumed to be a single digit.
(We don't need to calculate this, that warning already gives this
calculation).

Assumption is we won't be having more than 10 (again, no random number
usage for suffix, only single digit usage) variants of VCN FW that needs
to be loaded for a single SOC.

> 2. If you reduce the size of ucode_prefix to 25, it means that it can
> store a version string of up to 24 characters (since one character is
> needed for the null terminator).
> 
> For example: if tomorrow, if we get something like
> dimgrey_cavefish_smc_xxxyyyzz - then in this case yyyzz would be lost?
> is that in "amdgpu_ucode_ip_version_decode " &
> "amdgpu_ucode_legacy_naming" , is that are we always ensuring that it
> will never be longer than 24 characters?
> 

Newer FW naming follow the syntax vcn_xx.yy.zz (IP version), and that is
not more than 25 chars.

Anyway, you may use 50 for fw_name or adjust prefix size - both are fine.

Thanks,
Lijo

> Thanks,
> Srini
>>
>> With fw_name[42] also, you may run into 43 bytes (30 prefix + 13 for
>> others) warning.
>>
>> Thanks,
>> Lijo
>>
>>> -    char fw_name[40];
>>> +    char fw_name[42];
>>>       int r, i;
>>>         for (i = 0; i < adev->vcn.num_vcn_inst; i++) {

[PATCH v2] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[Srinivasan Shanmugam]

Reducing the size of ucode_prefix to 25 in the amdgpu_vcn_early_init
function. This would ensure that the total number of characters being
written into fw_name does not exceed its size of 40.

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                                                                  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                                                                  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
  102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive output may be truncated writing 4 bytes into a region of size between 2 and 31 [-Wformat-truncation=]
  105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
      |                                                                         ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 14 and 43 bytes into a destination of size 40
  105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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>
---
v2:
   - Reduced ucode_prefix instead of changing fw_name (Lijo)
   - updated commit message

 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9c514a606a2f..bb85772b1374 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -93,7 +93,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work);
 
 int amdgpu_vcn_early_init(struct amdgpu_device *adev)
 {
-	char ucode_prefix[30];
+	char ucode_prefix[25];
 	char fw_name[40];
 	int r, i;
 
-- 
2.34.1

Re: [PATCH v2] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

[Lazar, Lijo]

On 3/21/2024 10:29 AM, Srinivasan Shanmugam wrote:
> Reducing the size of ucode_prefix to 25 in the amdgpu_vcn_early_init
> function. This would ensure that the total number of characters being
> written into fw_name does not exceed its size of 40.
> 
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                                                                  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                                                                  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40
>   102 |                 snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive output may be truncated writing 4 bytes into a region of size between 2 and 31 [-Wformat-truncation=]
>   105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>       |                                                                         ^~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 14 and 43 bytes into a destination of size 40
>   105 |                         snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_%d.bin", ucode_prefix, i);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> 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

> ---
> v2:
>    - Reduced ucode_prefix instead of changing fw_name (Lijo)
>    - updated commit message
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 9c514a606a2f..bb85772b1374 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -93,7 +93,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work);
>  
>  int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>  {
> -	char ucode_prefix[30];
> +	char ucode_prefix[25];
>  	char fw_name[40];
>  	int r, i;
>