LinuxLists.cc - [PATCH v3] igb: Fix string truncation warnings in igb_set_fw

2024-01-15 08:28:56

Subject: [PATCH v3] igb: Fix string truncation warnings in igb_set_fw_version

Commit 1978d3ead82c ("intel: fix string truncation warnings")
fixes '-Wformat-truncation=' warnings in igb_main.c by using kasprintf.

drivers/net/ethernet/intel/igb/igb_main.c:3092:53: warning：‘%d’ directive output may be truncated writing between 1 and 5 bytes into a region of size between 1 and 13 [-Wformat-truncation=]
3092 | "%d.%d, 0x%08x, %d.%d.%d",
| ^~
drivers/net/ethernet/intel/igb/igb_main.c:3092:34: note：directive argument in the range [0, 65535]
3092 | "%d.%d, 0x%08x, %d.%d.%d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/igb/igb_main.c:3092:34: note：directive argument in the range [0, 65535]
drivers/net/ethernet/intel/igb/igb_main.c:3090:25: note：‘snprintf’ output between 23 and 43 bytes into a destination of size 32

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]/
v3: Add detailed warnings to the commit msg ,no functional change
---
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

2024-01-19 14:57:29

by Simon Horman

[permalink] [raw]

Subject: Re: [PATCH v3] igb: Fix string truncation warnings in igb_set_fw_version

On Mon, Jan 15, 2024 at 04:28:25PM +0800, Kunwu Chan wrote:
> Commit 1978d3ead82c ("intel: fix string truncation warnings")
> fixes '-Wformat-truncation=' warnings in igb_main.c by using kasprintf.
>
> drivers/net/ethernet/intel/igb/igb_main.c:3092:53: warning：‘%d’ directive output may be truncated writing between 1 and 5 bytes into a region of size between 1 and 13 [-Wformat-truncation=]
> 3092 | "%d.%d, 0x%08x, %d.%d.%d",
> | ^~
> drivers/net/ethernet/intel/igb/igb_main.c:3092:34: note：directive argument in the range [0, 65535]
> 3092 | "%d.%d, 0x%08x, %d.%d.%d",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/igb/igb_main.c:3092:34: note：directive argument in the range [0, 65535]
> drivers/net/ethernet/intel/igb/igb_main.c:3090:25: note：‘snprintf’ output between 23 and 43 bytes into a destination of size 32
>
> 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]>

Reviewed-by: Simon Horman <[email protected]>

2024-02-03 09:33:10

by Pucha, HimasekharX Reddy

[permalink] [raw]

Subject: RE: [Intel-wired-lan] [PATCH v3] igb: Fix string truncation warnings in igb_set_fw_version

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Kunwu Chan
> Sent: Monday, January 15, 2024 1:58 PM
> To: Brandeburg, Jesse <[email protected]>; Nguyen, Anthony L <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
> Cc: Kunwu Chan <[email protected]>; Kunwu Chan <[email protected]>; Kitszel, Przemyslaw <[email protected]>; [email protected]; [email protected]; [email protected]; Keller, Jacob E <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v3] igb: Fix string truncation warnings in igb_set_fw_version
>
> Commit 1978d3ead82c ("intel: fix string truncation warnings")
> fixes '-Wformat-truncation=' warnings in igb_main.c by using kasprintf.
>
> drivers/net/ethernet/intel/igb/igb_main.c:3092:53: warning：‘%d’ directive output may be truncated writing between 1 and 5 bytes into a region of size between 1 and 13 [-Wformat-truncation=]
> 3092 | "%d.%d, 0x%08x, %d.%d.%d",
> | ^~
> drivers/net/ethernet/intel/igb/igb_main.c:3092:34: note：directive argument in the range [0, 65535]
> 3092 | "%d.%d, 0x%08x, %d.%d.%d",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/igb/igb_main.c:3092:34: note：directive argument in the range [0, 65535]
> drivers/net/ethernet/intel/igb/igb_main.c:3090:25: note：‘snprintf’ output between 23 and 43 bytes into a destination of size 32
>
> 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]/
> v3: Add detailed warnings to the commit msg ,no functional change
> ---
> drivers/net/ethernet/intel/igb/igb.h | 2 +-
> drivers/net/ethernet/intel/igb/igb_main.c | 35 ++++++++++++-----------
> 2 files changed, 19 insertions(+), 18 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <[email protected]> (A Contingent worker at Intel)

2024-02-04 06:39:39

by Pucha, HimasekharX Reddy

[permalink] [raw]