[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2a58fed3-1fa9-47eb-b475-3f7c3b291376@molgen.mpg.de>
Date: Fri, 12 Jan 2024 14:46:42 +0100
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Kunwu Chan <chentao@...inos.cn>
Cc: jesse.brandeburg@...el.com, anthony.l.nguyen@...el.com,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, Kunwu Chan <kunwu.chan@...mail.com>,
przemyslaw.kitszel@...el.com, linux-kernel@...r.kernel.org,
intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
jacob.e.keller@...el.com
Subject: Re: [Intel-wired-lan] [PATCH v2] igb: Fix string truncation warnings
in igb_set_fw_version
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 <chentao@...inos.cn>
> Cc: Kunwu Chan <kunwu.chan@...mail.com>
> Suggested-by: Jakub Kicinski <kuba@...nel.org>
Kind regards,
Paul Menzel
> ---
> v2: Fall back to use snprintf and a larger space,as suggested by
> https://lore.kernel.org/all/20231212132637.1b0fb8aa@kernel.org/
> ---
> 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);
> }
>
> /**
Powered by blists - more mailing lists