Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
From:   Kalle Valo <kvalo@kernel.org>
To:     linux-wireless@vger.kernel.org
Subject: [PATCH RFC] wifi: iwlwifi: fix format-truncation warnings
Date:   Thu, 12 Oct 2023 17:02:51 +0300
Message-Id: <20231012140251.3473708-1-kvalo@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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