| Message ID | 20241216042843.work.498-kees@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [RESEND] wifi: iwlwifi: mvm: Fix __counted_by usage in cfg80211_wowlan_nd_* | expand |
> -----Original Message----- > From: Kees Cook <kees@kernel.org> > Sent: Monday, 16 December 2024 6:29 > To: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > Cc: Kees Cook <kees@kernel.org>; Gustavo A . R . Silva <gustavoars@kernel.org>; > Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; Kalle Valo > <kvalo@kernel.org>; Berg, Johannes <johannes.berg@intel.com>; Triebitz, Shaul > <shaul.triebitz@intel.com>; Grumbach, Emmanuel > <emmanuel.grumbach@intel.com>; Ben Shimol, Yedidya > <yedidya.ben.shimol@intel.com>; Berg, Benjamin <benjamin.berg@intel.com>; > Dmitry Antipov <dmantipov@yandex.ru>; linux-kernel@vger.kernel.org; linux- > wireless@vger.kernel.org; linux-hardening@vger.kernel.org > Subject: [PATCH RESEND] wifi: iwlwifi: mvm: Fix __counted_by usage in > cfg80211_wowlan_nd_* > > Both struct cfg80211_wowlan_nd_match and struct cfg80211_wowlan_nd_info > pre-allocate space for channels and matches, but then may end up using fewer > that the full allocation. Shrink the associated counter (n_channels and n_matches) > after counting the results. This avoids compile-time (and run-time) warnings from > __counted_by. (The counter member needs to be updated _before_ accessing the > array index.) > > Seen with coming GCC 15: > > drivers/net/wireless/intel/iwlwifi/mvm/d3.c: In function > 'iwl_mvm_query_set_freqs': > drivers/net/wireless/intel/iwlwifi/mvm/d3.c:2877:66: warning: operation on > 'match->n_channels' may be undefined [-Wsequence-point] > 2877 | match->channels[match->n_channels++] = > | ~~~~~~~~~~~~~~~~~^~ > drivers/net/wireless/intel/iwlwifi/mvm/d3.c:2885:66: warning: operation on > 'match->n_channels' may be undefined [-Wsequence-point] > 2885 | match->channels[match->n_channels++] = > | ~~~~~~~~~~~~~~~~~^~ > drivers/net/wireless/intel/iwlwifi/mvm/d3.c: In function > 'iwl_mvm_query_netdetect_reasons': > drivers/net/wireless/intel/iwlwifi/mvm/d3.c:2982:58: warning: operation on > 'net_detect->n_matches' may be undefined [-Wsequence-point] > 2982 | net_detect->matches[net_detect->n_matches++] = match; > | ~~~~~~~~~~~~~~~~~~~~~^~ > > Fixes: aa4ec06c455d ("wifi: cfg80211: use __counted_by where appropriate") > Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Link: https://lore.kernel.org/r/20240619211233.work.355-kees@kernel.org > Signed-off-by: Kees Cook <kees@kernel.org> > --- > Pinging this patch again, see > https://lore.kernel.org/lkml/20240619211233.work.355-kees@kernel.org/ > --- > drivers/net/wireless/intel/iwlwifi/mvm/d3.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c > b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c > index f85c01e04ebf..7d973546c9fb 100644 > --- a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c > @@ -2954,6 +2954,7 @@ static void iwl_mvm_query_set_freqs(struct iwl_mvm > *mvm, > int idx) > { > int i; > + int n_channels = 0; > > if (fw_has_api(&mvm->fw->ucode_capa, > IWL_UCODE_TLV_API_SCAN_OFFLOAD_CHANS)) { @@ - > 2962,7 +2963,7 @@ static void iwl_mvm_query_set_freqs(struct iwl_mvm > *mvm, > > for (i = 0; i < SCAN_OFFLOAD_MATCHING_CHANNELS_LEN * 8; > i++) > if (matches[idx].matching_channels[i / 8] & (BIT(i % 8))) > - match->channels[match->n_channels++] = > + match->channels[n_channels++] = > mvm->nd_channels[i]->center_freq; > } else { > struct iwl_scan_offload_profile_match_v1 *matches = @@ - > 2970,9 +2971,11 @@ static void iwl_mvm_query_set_freqs(struct iwl_mvm > *mvm, > > for (i = 0; i < SCAN_OFFLOAD_MATCHING_CHANNELS_LEN_V1 * > 8; i++) > if (matches[idx].matching_channels[i / 8] & (BIT(i % 8))) > - match->channels[match->n_channels++] = > + match->channels[n_channels++] = > mvm->nd_channels[i]->center_freq; > } > + /* We may have ended up with fewer channels than we allocated. */ > + match->n_channels = n_channels; > } > > /** > @@ -3053,6 +3056,8 @@ static void iwl_mvm_query_netdetect_reasons(struct > iwl_mvm *mvm, > GFP_KERNEL); > if (!net_detect || !n_matches) > goto out_report_nd; > + net_detect->n_matches = n_matches; > + n_matches = 0; > > for_each_set_bit(i, &matched_profiles, mvm->n_nd_match_sets) { > struct cfg80211_wowlan_nd_match *match; @@ -3066,8 > +3071,9 @@ static void iwl_mvm_query_netdetect_reasons(struct iwl_mvm > *mvm, > GFP_KERNEL); > if (!match) > goto out_report_nd; > + match->n_channels = n_channels; > > - net_detect->matches[net_detect->n_matches++] = match; > + net_detect->matches[n_matches++] = match; > > /* We inverted the order of the SSIDs in the scan > * request, so invert the index here. > @@ -3082,6 +3088,8 @@ static void iwl_mvm_query_netdetect_reasons(struct > iwl_mvm *mvm, > > iwl_mvm_query_set_freqs(mvm, d3_data->nd_results, match, i); > } > + /* We may have fewer matches than we allocated. */ > + net_detect->n_matches = n_matches; > > out_report_nd: > wakeup.net_detect = net_detect; > -- > 2.34.1 Hi , The patch was already applied and marked in Patchwork as such. Miri
On Mon, Dec 16, 2024 at 07:21:11AM +0000, Korenblit, Miriam Rachel wrote:
> The patch was already applied and marked in Patchwork as such.
Hi! Oh, I guess it hasn't made its way to -next yet? Thanks for
checking!
-Kees
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c index f85c01e04ebf..7d973546c9fb 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c @@ -2954,6 +2954,7 @@ static void iwl_mvm_query_set_freqs(struct iwl_mvm *mvm, int idx) { int i; + int n_channels = 0; if (fw_has_api(&mvm->fw->ucode_capa, IWL_UCODE_TLV_API_SCAN_OFFLOAD_CHANS)) { @@ -2962,7 +2963,7 @@ static void iwl_mvm_query_set_freqs(struct iwl_mvm *mvm, for (i = 0; i < SCAN_OFFLOAD_MATCHING_CHANNELS_LEN * 8; i++) if (matches[idx].matching_channels[i / 8] & (BIT(i % 8))) - match->channels[match->n_channels++] = + match->channels[n_channels++] = mvm->nd_channels[i]->center_freq; } else { struct iwl_scan_offload_profile_match_v1 *matches = @@ -2970,9 +2971,11 @@ static void iwl_mvm_query_set_freqs(struct iwl_mvm *mvm, for (i = 0; i < SCAN_OFFLOAD_MATCHING_CHANNELS_LEN_V1 * 8; i++) if (matches[idx].matching_channels[i / 8] & (BIT(i % 8))) - match->channels[match->n_channels++] = + match->channels[n_channels++] = mvm->nd_channels[i]->center_freq; } + /* We may have ended up with fewer channels than we allocated. */ + match->n_channels = n_channels; } /** @@ -3053,6 +3056,8 @@ static void iwl_mvm_query_netdetect_reasons(struct iwl_mvm *mvm, GFP_KERNEL); if (!net_detect || !n_matches) goto out_report_nd; + net_detect->n_matches = n_matches; + n_matches = 0; for_each_set_bit(i, &matched_profiles, mvm->n_nd_match_sets) { struct cfg80211_wowlan_nd_match *match; @@ -3066,8 +3071,9 @@ static void iwl_mvm_query_netdetect_reasons(struct iwl_mvm *mvm, GFP_KERNEL); if (!match) goto out_report_nd; + match->n_channels = n_channels; - net_detect->matches[net_detect->n_matches++] = match; + net_detect->matches[n_matches++] = match; /* We inverted the order of the SSIDs in the scan * request, so invert the index here. @@ -3082,6 +3088,8 @@ static void iwl_mvm_query_netdetect_reasons(struct iwl_mvm *mvm, iwl_mvm_query_set_freqs(mvm, d3_data->nd_results, match, i); } + /* We may have fewer matches than we allocated. */ + net_detect->n_matches = n_matches; out_report_nd: wakeup.net_detect = net_detect;
Both struct cfg80211_wowlan_nd_match and struct cfg80211_wowlan_nd_info pre-allocate space for channels and matches, but then may end up using fewer that the full allocation. Shrink the associated counter (n_channels and n_matches) after counting the results. This avoids compile-time (and run-time) warnings from __counted_by. (The counter member needs to be updated _before_ accessing the array index.) Seen with coming GCC 15: drivers/net/wireless/intel/iwlwifi/mvm/d3.c: In function 'iwl_mvm_query_set_freqs': drivers/net/wireless/intel/iwlwifi/mvm/d3.c:2877:66: warning: operation on 'match->n_channels' may be undefined [-Wsequence-point] 2877 | match->channels[match->n_channels++] = | ~~~~~~~~~~~~~~~~~^~ drivers/net/wireless/intel/iwlwifi/mvm/d3.c:2885:66: warning: operation on 'match->n_channels' may be undefined [-Wsequence-point] 2885 | match->channels[match->n_channels++] = | ~~~~~~~~~~~~~~~~~^~ drivers/net/wireless/intel/iwlwifi/mvm/d3.c: In function 'iwl_mvm_query_netdetect_reasons': drivers/net/wireless/intel/iwlwifi/mvm/d3.c:2982:58: warning: operation on 'net_detect->n_matches' may be undefined [-Wsequence-point] 2982 | net_detect->matches[net_detect->n_matches++] = match; | ~~~~~~~~~~~~~~~~~~~~~^~ Fixes: aa4ec06c455d ("wifi: cfg80211: use __counted_by where appropriate") Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Link: https://lore.kernel.org/r/20240619211233.work.355-kees@kernel.org Signed-off-by: Kees Cook <kees@kernel.org> --- Pinging this patch again, see https://lore.kernel.org/lkml/20240619211233.work.355-kees@kernel.org/ --- drivers/net/wireless/intel/iwlwifi/mvm/d3.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)