public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Cc: Kees Cook <kees@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Johannes Berg <johannes.berg@intel.com>,
	Yedidya Benshimol <yedidya.ben.shimol@intel.com>,
	Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
	Avraham Stern <avraham.stern@intel.com>,
	linux-wireless@vger.kernel.org,
	Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	Benjamin Berg <benjamin.berg@intel.com>,
	Daniel Gabay <daniel.gabay@intel.com>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
	linux-hardening@vger.kernel.org
Subject: [PATCH v2] wifi: iwlwifi: mld: Work around Clang loop unrolling bug
Date: Fri, 25 Apr 2025 11:44:22 -0700	[thread overview]
Message-ID: <20250425184418.it.308-kees@kernel.org> (raw)

The nested loop in iwl_mld_send_proto_offload() confuses Clang into
thinking there could be final loop iteration past the end of the "nsc"
array (which is only 4 entries). The FORTIFY checking in memcmp()
(via ipv6_addr_cmp()) notices this (due to the available bytes in the
out-of-bounds position of &nsc[4] being 0), and errors out, failing
the build. For some reason (likely due to architectural loop unrolling
configurations), this is only exposed on ARM builds currently. Due to
Clang's lack of inline tracking[1], the warning is not very helpful:

include/linux/fortify-string.h:719:4: error: call to '__read_overflow' declared with 'error' attribute: detected read beyond size of object (1st parameter)
  719 |                         __read_overflow();
      |                         ^
1 error generated.

But this was tracked down to iwl_mld_send_proto_offload()'s
ipv6_addr_cmp() call.

An upstream Clang bug has been filed[2] to track this, but for now.
Fix the build by explicitly bounding the inner loop by "n_nsc", which is
what "c" is already limited to. Additionally do not repeat the ns_config
and targ_addrs array sizes with their open-coded names since they can
be determined at compile-time with ARRAY_SIZE().

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://github.com/ClangBuiltLinux/linux/issues/2076
Link: https://github.com/llvm/llvm-project/pull/73552 [1]
Link: https://github.com/llvm/llvm-project/issues/136603 [2]
Signed-off-by: Kees Cook <kees@kernel.org>
---
 v2:
  - move "j < n_nsc" forward to stabilize loop bounds (Nathan)
  - use ARRAY_SIZE() for robustness
 v1: https://lore.kernel.org/all/20250421204153.work.935-kees@kernel.org/
Cc: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Yedidya Benshimol <yedidya.ben.shimol@intel.com>
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Cc: Avraham Stern <avraham.stern@intel.com>
Cc: <linux-wireless@vger.kernel.org>
---
 drivers/net/wireless/intel/iwlwifi/mld/d3.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mld/d3.c b/drivers/net/wireless/intel/iwlwifi/mld/d3.c
index dc736fdc176d..c51a6596617d 100644
--- a/drivers/net/wireless/intel/iwlwifi/mld/d3.c
+++ b/drivers/net/wireless/intel/iwlwifi/mld/d3.c
@@ -1728,17 +1728,13 @@ iwl_mld_send_proto_offload(struct iwl_mld *mld,
 #if IS_ENABLED(CONFIG_IPV6)
 	struct iwl_mld_vif *mld_vif = iwl_mld_vif_from_mac80211(vif);
 	struct iwl_mld_wowlan_data *wowlan_data = &mld_vif->wowlan_data;
-	struct iwl_ns_config *nsc;
-	struct iwl_targ_addr *addrs;
-	int n_nsc, n_addrs;
+	const int n_addrs = ARRAY_SIZE(cmd->targ_addrs);
+	struct iwl_targ_addr *addrs = cmd->targ_addrs;
+	const int n_nsc = ARRAY_SIZE(cmd->ns_config);
+	struct iwl_ns_config *nsc = cmd->ns_config;
 	int i, c;
 	int num_skipped = 0;
 
-	nsc = cmd->ns_config;
-	n_nsc = IWL_PROTO_OFFLOAD_NUM_NS_CONFIG_V3L;
-	addrs = cmd->targ_addrs;
-	n_addrs = IWL_PROTO_OFFLOAD_NUM_IPV6_ADDRS_V3L;
-
 	/* For each address we have (and that will fit) fill a target
 	 * address struct and combine for NS offload structs with the
 	 * solicited node addresses.
@@ -1759,7 +1755,7 @@ iwl_mld_send_proto_offload(struct iwl_mld *mld,
 
 		addrconf_addr_solict_mult(&wowlan_data->target_ipv6_addrs[i],
 					  &solicited_addr);
-		for (j = 0; j < c; j++)
+		for (j = 0; j < n_nsc && j < c; j++)
 			if (ipv6_addr_cmp(&nsc[j].dest_ipv6_addr,
 					  &solicited_addr) == 0)
 				break;
-- 
2.34.1


             reply	other threads:[~2025-04-25 18:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25 18:44 Kees Cook [this message]
2025-04-26  0:04 ` [PATCH v2] wifi: iwlwifi: mld: Work around Clang loop unrolling bug Nathan Chancellor
2025-05-20 17:23 ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250425184418.it.308-kees@kernel.org \
    --to=kees@kernel.org \
    --cc=avraham.stern@intel.com \
    --cc=benjamin.berg@intel.com \
    --cc=daniel.gabay@intel.com \
    --cc=emmanuel.grumbach@intel.com \
    --cc=johannes.berg@intel.com \
    --cc=justinstitt@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=miriam.rachel.korenblit@intel.com \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=yedidya.ben.shimol@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox