'commit 1978d3ead82c ("intel: fix string truncation warnings")'
fix '-Wformat-truncation=' warnings in igb_main.c by using kasprintf.
kasprintf() returns a pointer to dynamically allocated memory
which can be NULL upon failure.
Fix this warning by using a larger space for adapter->fw_version,
and then fall back and continue to use snprintf.
Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
Signed-off-by: Kunwu Chan <[email protected]>
Cc: Kunwu Chan <[email protected]>
Suggested-by: Jakub Kicinski <[email protected]>
---
v2: Fall back to use snprintf and a larger space,as suggested by
https://lore.kernel.org/all/[email protected]/
---
drivers/net/ethernet/intel/igb/igb.h | 2 +-
drivers/net/ethernet/intel/igb/igb_main.c | 35 ++++++++++++-----------
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index a2b759531cb7..3c2dc7bdebb5 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -637,7 +637,7 @@ struct igb_adapter {
struct timespec64 period;
} perout[IGB_N_PEROUT];
- char fw_version[32];
+ char fw_version[48];
#ifdef CONFIG_IGB_HWMON
struct hwmon_buff *igb_hwmon_buff;
bool ets;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b2295caa2f0a..ce762d77d2c1 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3069,7 +3069,6 @@ void igb_set_fw_version(struct igb_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
struct e1000_fw_version fw;
- char *lbuf;
igb_get_fw_version(hw, &fw);
@@ -3077,34 +3076,36 @@ void igb_set_fw_version(struct igb_adapter *adapter)
case e1000_i210:
case e1000_i211:
if (!(igb_get_flash_presence_i210(hw))) {
- lbuf = kasprintf(GFP_KERNEL, "%2d.%2d-%d",
- fw.invm_major, fw.invm_minor,
- fw.invm_img_type);
+ snprintf(adapter->fw_version,
+ sizeof(adapter->fw_version),
+ "%2d.%2d-%d",
+ fw.invm_major, fw.invm_minor,
+ fw.invm_img_type);
break;
}
fallthrough;
default:
/* if option rom is valid, display its version too */
if (fw.or_valid) {
- lbuf = kasprintf(GFP_KERNEL, "%d.%d, 0x%08x, %d.%d.%d",
- fw.eep_major, fw.eep_minor,
- fw.etrack_id, fw.or_major, fw.or_build,
- fw.or_patch);
+ snprintf(adapter->fw_version,
+ sizeof(adapter->fw_version),
+ "%d.%d, 0x%08x, %d.%d.%d",
+ fw.eep_major, fw.eep_minor, fw.etrack_id,
+ fw.or_major, fw.or_build, fw.or_patch);
/* no option rom */
} else if (fw.etrack_id != 0X0000) {
- lbuf = kasprintf(GFP_KERNEL, "%d.%d, 0x%08x",
- fw.eep_major, fw.eep_minor,
- fw.etrack_id);
+ snprintf(adapter->fw_version,
+ sizeof(adapter->fw_version),
+ "%d.%d, 0x%08x",
+ fw.eep_major, fw.eep_minor, fw.etrack_id);
} else {
- lbuf = kasprintf(GFP_KERNEL, "%d.%d.%d", fw.eep_major,
- fw.eep_minor, fw.eep_build);
+ snprintf(adapter->fw_version,
+ sizeof(adapter->fw_version),
+ "%d.%d.%d",
+ fw.eep_major, fw.eep_minor, fw.eep_build);
}
break;
}
-
- /* the truncate happens here if it doesn't fit */
- strscpy(adapter->fw_version, lbuf, sizeof(adapter->fw_version));
- kfree(lbuf);
}
/**
--
2.39.2
Dear Kunwu,
Thank you for your patch. I have some minor nits.
Am 12.01.24 um 03:58 schrieb Kunwu Chan:
> 'commit 1978d3ead82c ("intel: fix string truncation warnings")'
Please don’t enclose it in '': Commit 1978d3ead82c ("intel: fix string
truncation warnings")
> fix '-Wformat-truncation=' warnings in igb_main.c by using kasprintf.
fix*es*
> kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure.
Maybe paste one warning message.
> Fix this warning by using a larger space for adapter->fw_version,
> and then fall back and continue to use snprintf.
>
> Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
> Signed-off-by: Kunwu Chan <[email protected]>
> Cc: Kunwu Chan <[email protected]>
> Suggested-by: Jakub Kicinski <[email protected]>
Kind regards,
Paul Menzel
> ---
> v2: Fall back to use snprintf and a larger space,as suggested by
> https://lore.kernel.org/all/[email protected]/
> ---
> drivers/net/ethernet/intel/igb/igb.h | 2 +-
> drivers/net/ethernet/intel/igb/igb_main.c | 35 ++++++++++++-----------
> 2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> index a2b759531cb7..3c2dc7bdebb5 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -637,7 +637,7 @@ struct igb_adapter {
> struct timespec64 period;
> } perout[IGB_N_PEROUT];
>
> - char fw_version[32];
> + char fw_version[48];
> #ifdef CONFIG_IGB_HWMON
> struct hwmon_buff *igb_hwmon_buff;
> bool ets;
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index b2295caa2f0a..ce762d77d2c1 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3069,7 +3069,6 @@ void igb_set_fw_version(struct igb_adapter *adapter)
> {
> struct e1000_hw *hw = &adapter->hw;
> struct e1000_fw_version fw;
> - char *lbuf;
>
> igb_get_fw_version(hw, &fw);
>
> @@ -3077,34 +3076,36 @@ void igb_set_fw_version(struct igb_adapter *adapter)
> case e1000_i210:
> case e1000_i211:
> if (!(igb_get_flash_presence_i210(hw))) {
> - lbuf = kasprintf(GFP_KERNEL, "%2d.%2d-%d",
> - fw.invm_major, fw.invm_minor,
> - fw.invm_img_type);
> + snprintf(adapter->fw_version,
> + sizeof(adapter->fw_version),
> + "%2d.%2d-%d",
> + fw.invm_major, fw.invm_minor,
> + fw.invm_img_type);
> break;
> }
> fallthrough;
> default:
> /* if option rom is valid, display its version too */
> if (fw.or_valid) {
> - lbuf = kasprintf(GFP_KERNEL, "%d.%d, 0x%08x, %d.%d.%d",
> - fw.eep_major, fw.eep_minor,
> - fw.etrack_id, fw.or_major, fw.or_build,
> - fw.or_patch);
> + snprintf(adapter->fw_version,
> + sizeof(adapter->fw_version),
> + "%d.%d, 0x%08x, %d.%d.%d",
> + fw.eep_major, fw.eep_minor, fw.etrack_id,
> + fw.or_major, fw.or_build, fw.or_patch);
> /* no option rom */
> } else if (fw.etrack_id != 0X0000) {
> - lbuf = kasprintf(GFP_KERNEL, "%d.%d, 0x%08x",
> - fw.eep_major, fw.eep_minor,
> - fw.etrack_id);
> + snprintf(adapter->fw_version,
> + sizeof(adapter->fw_version),
> + "%d.%d, 0x%08x",
> + fw.eep_major, fw.eep_minor, fw.etrack_id);
> } else {
> - lbuf = kasprintf(GFP_KERNEL, "%d.%d.%d", fw.eep_major,
> - fw.eep_minor, fw.eep_build);
> + snprintf(adapter->fw_version,
> + sizeof(adapter->fw_version),
> + "%d.%d.%d",
> + fw.eep_major, fw.eep_minor, fw.eep_build);
> }
> break;
> }
> -
> - /* the truncate happens here if it doesn't fit */
> - strscpy(adapter->fw_version, lbuf, sizeof(adapter->fw_version));
> - kfree(lbuf);
> }
>
> /**
Hi Paul,
Thanks for your reply.
On 2024/1/12 21:46, Paul Menzel wrote:
> Dear Kunwu,
>
>
> Thank you for your patch. I have some minor nits.
>
> Am 12.01.24 um 03:58 schrieb Kunwu Chan:
>> 'commit 1978d3ead82c ("intel: fix string truncation warnings")'
>
> Please don’t enclose it in '': Commit 1978d3ead82c ("intel: fix string
> truncation warnings")
>
>> fix '-Wformat-truncation=' warnings in igb_main.c by using kasprintf.
>
> fix*es*
Thanks i'll rewirte the commit msg and add some warnings.
>
>> kasprintf() returns a pointer to dynamically allocated memory
>> which can be NULL upon failure.
>
> Maybe paste one warning message.
Thanks again.
>
>> Fix this warning by using a larger space for adapter->fw_version,
>> and then fall back and continue to use snprintf.
>>
>> Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
>> Signed-off-by: Kunwu Chan <[email protected]>
>> Cc: Kunwu Chan <[email protected]>
>> Suggested-by: Jakub Kicinski <[email protected]>
>
>
> Kind regards,
>
> Paul Menzel
>
>
>> ---
>> v2: Fall back to use snprintf and a larger space,as suggested by
>> https://lore.kernel.org/all/[email protected]/
>> ---
>> drivers/net/ethernet/intel/igb/igb.h | 2 +-
>> drivers/net/ethernet/intel/igb/igb_main.c | 35 ++++++++++++-----------
>> 2 files changed, 19 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb.h
>> b/drivers/net/ethernet/intel/igb/igb.h
>> index a2b759531cb7..3c2dc7bdebb5 100644
>> --- a/drivers/net/ethernet/intel/igb/igb.h
>> +++ b/drivers/net/ethernet/intel/igb/igb.h
>> @@ -637,7 +637,7 @@ struct igb_adapter {
>> struct timespec64 period;
>> } perout[IGB_N_PEROUT];
>> - char fw_version[32];
>> + char fw_version[48];
>> #ifdef CONFIG_IGB_HWMON
>> struct hwmon_buff *igb_hwmon_buff;
>> bool ets;
>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
>> b/drivers/net/ethernet/intel/igb/igb_main.c
>> index b2295caa2f0a..ce762d77d2c1 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -3069,7 +3069,6 @@ void igb_set_fw_version(struct igb_adapter
>> *adapter)
>> {
>> struct e1000_hw *hw = &adapter->hw;
>> struct e1000_fw_version fw;
>> - char *lbuf;
>> igb_get_fw_version(hw, &fw);
>> @@ -3077,34 +3076,36 @@ void igb_set_fw_version(struct igb_adapter
>> *adapter)
>> case e1000_i210:
>> case e1000_i211:
>> if (!(igb_get_flash_presence_i210(hw))) {
>> - lbuf = kasprintf(GFP_KERNEL, "%2d.%2d-%d",
>> - fw.invm_major, fw.invm_minor,
>> - fw.invm_img_type);
>> + snprintf(adapter->fw_version,
>> + sizeof(adapter->fw_version),
>> + "%2d.%2d-%d",
>> + fw.invm_major, fw.invm_minor,
>> + fw.invm_img_type);
>> break;
>> }
>> fallthrough;
>> default:
>> /* if option rom is valid, display its version too */
>> if (fw.or_valid) {
>> - lbuf = kasprintf(GFP_KERNEL, "%d.%d, 0x%08x, %d.%d.%d",
>> - fw.eep_major, fw.eep_minor,
>> - fw.etrack_id, fw.or_major, fw.or_build,
>> - fw.or_patch);
>> + snprintf(adapter->fw_version,
>> + sizeof(adapter->fw_version),
>> + "%d.%d, 0x%08x, %d.%d.%d",
>> + fw.eep_major, fw.eep_minor, fw.etrack_id,
>> + fw.or_major, fw.or_build, fw.or_patch);
>> /* no option rom */
>> } else if (fw.etrack_id != 0X0000) {
>> - lbuf = kasprintf(GFP_KERNEL, "%d.%d, 0x%08x",
>> - fw.eep_major, fw.eep_minor,
>> - fw.etrack_id);
>> + snprintf(adapter->fw_version,
>> + sizeof(adapter->fw_version),
>> + "%d.%d, 0x%08x",
>> + fw.eep_major, fw.eep_minor, fw.etrack_id);
>> } else {
>> - lbuf = kasprintf(GFP_KERNEL, "%d.%d.%d", fw.eep_major,
>> - fw.eep_minor, fw.eep_build);
>> + snprintf(adapter->fw_version,
>> + sizeof(adapter->fw_version),
>> + "%d.%d.%d",
>> + fw.eep_major, fw.eep_minor, fw.eep_build);
>> }
>> break;
>> }
>> -
>> - /* the truncate happens here if it doesn't fit */
>> - strscpy(adapter->fw_version, lbuf, sizeof(adapter->fw_version));
>> - kfree(lbuf);
>> }
>> /**
--
Thanks,
Kunwu