linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] wifi: iwlwifi: fix format-truncation warnings
@ 2023-10-12 14:02 Kalle Valo
  2023-10-12 15:57 ` Kalle Valo
  0 siblings, 1 reply; 2+ messages in thread
From: Kalle Valo @ 2023-10-12 14:02 UTC (permalink / raw)
  To: linux-wireless

On v6.6-rc4 with GCC 13.2 I see:

drivers/net/wireless/intel/iwlwifi/dvm/main.c:1467:19: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size 32 [-Wformat-truncation=]
drivers/net/wireless/intel/iwlwifi/dvm/main.c:1465:9: note: 'snprintf' output between 1 and 64 bytes into a destination of size 32
drivers/net/wireless/intel/iwlwifi/mvm/ops.c:1307:19: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size 32 [-Wformat-truncation=]
drivers/net/wireless/intel/iwlwifi/mvm/ops.c:1305:9: note: 'snprintf' output between 1 and 64 bytes into a destination of size 32
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:549:33: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size between 48 and 56 [-Wformat-truncation=]
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:547:9: note: 'snprintf' output 9 or more bytes (assuming 80) into a destination of size 64
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:729:33: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size between 48 and 56 [-Wformat-truncation=]
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:727:9: note: 'snprintf' output 9 or more bytes (assuming 80) into a destination of size 64
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:989:51: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size between 46 and 58 [-Wformat-truncation=]
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:987:33: note: 'snprintf' output between 7 and 82 bytes into a destination of size 64
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:984:53: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size between 40 and 50 [-Wformat-truncation=]
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:982:33: note: 'snprintf' output between 15 and 88 bytes into a destination of size 64

Two of the warnings were easy to fix by using strscpy(). But the rest were more
challening. For now I was only able to come up with artificial testing of
snprintf() return value but that doesn't make really sense. I marked the ugly
once "FIXME" in the code.

Any ideas how to fix the warnings properly?

Compile tested only.

Signed-off-by: Kalle Valo <kvalo@kernel.org>
---
 drivers/net/wireless/intel/iwlwifi/dvm/main.c |  5 +-
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c  | 64 +++++++++++--------
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c  |  5 +-
 3 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/main.c b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
index a873be109f43..6b252f19b641 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/main.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
@@ -1462,9 +1462,8 @@ static struct iwl_op_mode *iwl_op_mode_dvm_start(struct iwl_trans *trans,
 	iwl_power_initialize(priv);
 	iwl_tt_initialize(priv);
 
-	snprintf(priv->hw->wiphy->fw_version,
-		 sizeof(priv->hw->wiphy->fw_version),
-		 "%s", fw->fw_version);
+	strscpy(priv->hw->wiphy->fw_version, fw->fw_version,
+		sizeof(priv->hw->wiphy->fw_version));
 
 	priv->new_scan_threshold_behaviour =
 		!!(ucode_flags & IWL_UCODE_TLV_FLAGS_NEWSCAN);
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index fb5e254757e7..85cb7ad604d5 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -496,6 +496,7 @@ static int iwl_parse_v1_v2_firmware(struct iwl_drv *drv,
 	u32 api_ver, hdr_size, build;
 	char buildstr[25];
 	const u8 *src;
+	int len;
 
 	drv->fw.ucode_ver = le32_to_cpu(ucode->ver);
 	api_ver = IWL_UCODE_API(drv->fw.ucode_ver);
@@ -544,14 +545,16 @@ static int iwl_parse_v1_v2_firmware(struct iwl_drv *drv,
 	else
 		buildstr[0] = '\0';
 
-	snprintf(drv->fw.fw_version,
-		 sizeof(drv->fw.fw_version),
-		 "%u.%u.%u.%u%s %s",
-		 IWL_UCODE_MAJOR(drv->fw.ucode_ver),
-		 IWL_UCODE_MINOR(drv->fw.ucode_ver),
-		 IWL_UCODE_API(drv->fw.ucode_ver),
-		 IWL_UCODE_SERIAL(drv->fw.ucode_ver),
-		 buildstr, iwl_reduced_fw_name(drv));
+	len = snprintf(drv->fw.fw_version,
+		       sizeof(drv->fw.fw_version),
+		       "%u.%u.%u.%u%s %s",
+		       IWL_UCODE_MAJOR(drv->fw.ucode_ver),
+		       IWL_UCODE_MINOR(drv->fw.ucode_ver),
+		       IWL_UCODE_API(drv->fw.ucode_ver),
+		       IWL_UCODE_SERIAL(drv->fw.ucode_ver),
+		       buildstr, iwl_reduced_fw_name(drv));
+	if (len)
+		printk("FIXME");
 
 	/* Verify size of file vs. image size info in file's header */
 
@@ -700,7 +703,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
 	const u8 *tlv_data;
 	char buildstr[25];
 	u32 build, paging_mem_size;
-	int num_of_cpus;
+	int num_of_cpus, l;
 	bool usniffer_req = false;
 
 	if (len < sizeof(*ucode)) {
@@ -724,14 +727,16 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
 	else
 		buildstr[0] = '\0';
 
-	snprintf(drv->fw.fw_version,
-		 sizeof(drv->fw.fw_version),
-		 "%u.%u.%u.%u%s %s",
-		 IWL_UCODE_MAJOR(drv->fw.ucode_ver),
-		 IWL_UCODE_MINOR(drv->fw.ucode_ver),
-		 IWL_UCODE_API(drv->fw.ucode_ver),
-		 IWL_UCODE_SERIAL(drv->fw.ucode_ver),
-		 buildstr, iwl_reduced_fw_name(drv));
+	l = snprintf(drv->fw.fw_version,
+		       sizeof(drv->fw.fw_version),
+		       "%u.%u.%u.%u%s %s",
+		       IWL_UCODE_MAJOR(drv->fw.ucode_ver),
+		       IWL_UCODE_MINOR(drv->fw.ucode_ver),
+		       IWL_UCODE_API(drv->fw.ucode_ver),
+		       IWL_UCODE_SERIAL(drv->fw.ucode_ver),
+		       buildstr, iwl_reduced_fw_name(drv));
+	if (l)
+		printk("FIXME");
 
 	data = ucode->data;
 
@@ -978,16 +983,21 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
 			minor = le32_to_cpup(ptr++);
 			local_comp = le32_to_cpup(ptr);
 
-			if (major >= 35)
-				snprintf(drv->fw.fw_version,
-					 sizeof(drv->fw.fw_version),
-					"%u.%08x.%u %s", major, minor,
-					local_comp, iwl_reduced_fw_name(drv));
-			else
-				snprintf(drv->fw.fw_version,
-					 sizeof(drv->fw.fw_version),
-					"%u.%u.%u %s", major, minor,
-					local_comp, iwl_reduced_fw_name(drv));
+			if (major >= 35) {
+				l = snprintf(drv->fw.fw_version,
+					     sizeof(drv->fw.fw_version),
+					     "%u.%08x.%u %s", major, minor,
+					     local_comp, iwl_reduced_fw_name(drv));
+				if (l)
+					printk("FIXME");
+			} else {
+				l = snprintf(drv->fw.fw_version,
+					     sizeof(drv->fw.fw_version),
+					     "%u.%u.%u %s", major, minor,
+					     local_comp, iwl_reduced_fw_name(drv));
+				if (l)
+					printk("FIXME");
+			}
 			break;
 			}
 		case IWL_UCODE_TLV_FW_DBG_DEST: {
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index 465090f67aaf..3864aaf7c1ac 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -1302,9 +1302,8 @@ iwl_op_mode_mvm_start(struct iwl_trans *trans, const struct iwl_cfg *cfg,
 	trans_cfg.cmd_q_wdg_timeout =
 		iwl_mvm_get_wd_timeout(mvm, NULL, false, true);
 
-	snprintf(mvm->hw->wiphy->fw_version,
-		 sizeof(mvm->hw->wiphy->fw_version),
-		 "%s", fw->fw_version);
+	strscpy(mvm->hw->wiphy->fw_version, fw->fw_version,
+		sizeof(mvm->hw->wiphy->fw_version));
 
 	trans_cfg.fw_reset_handshake = fw_has_capa(&mvm->fw->ucode_capa,
 						   IWL_UCODE_TLV_CAPA_FW_RESET_HANDSHAKE);

base-commit: 618071ae0f7e8c1aeb9b1b1e9ee876bcc3f045fc
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH RFC] wifi: iwlwifi: fix format-truncation warnings
  2023-10-12 14:02 [PATCH RFC] wifi: iwlwifi: fix format-truncation warnings Kalle Valo
@ 2023-10-12 15:57 ` Kalle Valo
  0 siblings, 0 replies; 2+ messages in thread
From: Kalle Valo @ 2023-10-12 15:57 UTC (permalink / raw)
  To: linux-wireless

Kalle Valo <kvalo@kernel.org> writes:

> On v6.6-rc4 with GCC 13.2 I see:
>
> drivers/net/wireless/intel/iwlwifi/dvm/main.c:1467:19: warning: '%s'
> directive output may be truncated writing up to 63 bytes into a region
> of size 32 [-Wformat-truncation=]
> drivers/net/wireless/intel/iwlwifi/dvm/main.c:1465:9: note: 'snprintf'
> output between 1 and 64 bytes into a destination of size 32
> drivers/net/wireless/intel/iwlwifi/mvm/ops.c:1307:19: warning: '%s'
> directive output may be truncated writing up to 63 bytes into a region
> of size 32 [-Wformat-truncation=]
> drivers/net/wireless/intel/iwlwifi/mvm/ops.c:1305:9: note: 'snprintf'
> output between 1 and 64 bytes into a destination of size 32
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:549:33: warning: '%s'
> directive output may be truncated writing up to 63 bytes into a region
> of size between 48 and 56 [-Wformat-truncation=]
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:547:9: note: 'snprintf'
> output 9 or more bytes (assuming 80) into a destination of size 64
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:729:33: warning: '%s'
> directive output may be truncated writing up to 63 bytes into a region
> of size between 48 and 56 [-Wformat-truncation=]
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:727:9: note: 'snprintf'
> output 9 or more bytes (assuming 80) into a destination of size 64
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:989:51: warning: '%s'
> directive output may be truncated writing up to 63 bytes into a region
> of size between 46 and 58 [-Wformat-truncation=]
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:987:33: note: 'snprintf'
> output between 7 and 82 bytes into a destination of size 64
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:984:53: warning: '%s'
> directive output may be truncated writing up to 63 bytes into a region
> of size between 40 and 50 [-Wformat-truncation=]
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:982:33: note: 'snprintf'
> output between 15 and 88 bytes into a destination of size 64
>
> Two of the warnings were easy to fix by using strscpy(). But the rest were more
> challening. For now I was only able to come up with artificial testing of
> snprintf() return value but that doesn't make really sense. I marked the ugly
> once "FIXME" in the code.
>
> Any ideas how to fix the warnings properly?
>
> Compile tested only.
>
> Signed-off-by: Kalle Valo <kvalo@kernel.org>

I didn't notice that Gregory had already submitted a patch:

https://patchwork.kernel.org/project/linux-wireless/patch/20231012153950.f4465b4b4e2b.Idced2e8d63c492872edcde1a3ce2cdd6cc0f8eb7@changeid/

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-10-12 15:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-12 14:02 [PATCH RFC] wifi: iwlwifi: fix format-truncation warnings Kalle Valo
2023-10-12 15:57 ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).