On Tue, Apr 22, 2025 at 12:59:03PM -0700, Nathan Chancellor wrote: > On Mon, Apr 21, 2025 at 01:41:57PM -0700, Kees Cook wrote: > > 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. > > > > Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx> > > 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@xxxxxxxxxx> > > --- > > Cc: Miri Korenblit <miriam.rachel.korenblit@xxxxxxxxx> > > Cc: Johannes Berg <johannes.berg@xxxxxxxxx> > > Cc: Yedidya Benshimol <yedidya.ben.shimol@xxxxxxxxx> > > Cc: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx> > > Cc: Avraham Stern <avraham.stern@xxxxxxxxx> > > Cc: Daniel Gabay <daniel.gabay@xxxxxxxxx> > > Cc: <linux-wireless@xxxxxxxxxxxxxxx> > > --- > > drivers/net/wireless/intel/iwlwifi/mld/d3.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/intel/iwlwifi/mld/d3.c b/drivers/net/wireless/intel/iwlwifi/mld/d3.c > > index 2c6e8ecd93b7..1daca1ef02b2 100644 > > --- a/drivers/net/wireless/intel/iwlwifi/mld/d3.c > > +++ b/drivers/net/wireless/intel/iwlwifi/mld/d3.c > > @@ -1754,7 +1754,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 < c && j < n_nsc; j++) > > if (ipv6_addr_cmp(&nsc[j].dest_ipv6_addr, > > &solicited_addr) == 0) > > break; > > -- > > 2.34.1 > > > > I might be going crazy but this does not appear to actually resolve the > warning for me, at least with allmodconfig: > > $ git cite > a33b5a08cbbd ("Merge tag 'sched_ext-for-6.15-rc3-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext") Make me look. :) "cite" is a local alias, yes? Looks like my own alias for "short", but basically "short HEAD". From my ~/.gitconfig: [alias] short = "!f() { for i in \"$@\"; do git log -1 --pretty='%h (\"%s\")' \"$i\"; done; }; f" > diff --git a/drivers/net/wireless/intel/iwlwifi/mld/d3.c b/drivers/net/wireless/intel/iwlwifi/mld/d3.c > index fba6a7b1bb5c..7ce01ad3608e 100644 > --- a/drivers/net/wireless/intel/iwlwifi/mld/d3.c > +++ b/drivers/net/wireless/intel/iwlwifi/mld/d3.c > @@ -1757,7 +1757,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 < n_nsc; j++) > + for (j = 0; j < n_nsc && j < c; j++) > if (ipv6_addr_cmp(&nsc[j].dest_ipv6_addr, > &solicited_addr) == 0) > break; Oof, an unstable solution. Well, I guess we work with what we've got. Your change also solves it for me, so I'll send a v2 with it that way. On the "getting it fixed correctly" front, we need someone who can tweak SCEV: https://github.com/llvm/llvm-project/issues/136603 -Kees -- Kees Cook
