| Message ID | 20240102213313.4cb3dbb1d84f.I7c74edec83c5d7598cdd578929fd0876d67aef7f@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | cfg80211/mac80211 patches from our internal tree 2024-01-02 | expand |
Hi, just noticed this patch has an off-by-one error and it drops the last byte of the generated RNR. On Tue, 2024-01-02 at 21:35 +0200, Miri Korenblit wrote: > From: Benjamin Berg <benjamin.berg@intel.com> > > If the reporting AP is part of the same MLD, then an entry in the RNR > is > required in order to discover it again from the BSS generated from > the > per-STA profile in the Multi-Link Probe Response. > > We need this because we do not have a direct concept of an MLD AP and > just do the lookup from one to the other on the fly if needed. As > such, > we need to ensure that this lookup will work both ways. > > Fixes: 2481b5da9c6b ("wifi: cfg80211: handle BSS data contained in ML > probe responses") > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com> > Reviewed-by: Johannes Berg <johannes.berg@intel.com> > Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com> > --- > net/wireless/scan.c | 134 > ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 129 insertions(+), 5 deletions(-) > > diff --git a/net/wireless/scan.c b/net/wireless/scan.c > index 42555753b947..dbb5885d40e7 100644 > --- a/net/wireless/scan.c > +++ b/net/wireless/scan.c > @@ -2614,6 +2614,103 @@ cfg80211_tbtt_info_for_mld_ap(const u8 *ie, > size_t ielen, u8 mld_id, u8 link_id, > return 0; > } > > +static struct element * > +cfg80211_gen_reporter_rnr(struct cfg80211_bss *source_bss, bool > is_mbssid, > + bool same_mld, u8 link_id, u8 > bss_change_count, > + gfp_t gfp) > +{ > + const struct cfg80211_bss_ies *ies; > + struct ieee80211_neighbor_ap_info ap_info; > + struct ieee80211_tbtt_info_ge_11 tbtt_info; > + u32 short_ssid; > + const struct element *elem; > + struct element *res; > + > + /* > + * We only generate the RNR to permit ML lookups. For that we > do not > + * need an entry for the corresponding transmitting BSS, lets > just skip > + * it even though it would be easy to add. > + */ > + if (!same_mld) > + return NULL; > + > + /* We could use tx_data->ies if we change > cfg80211_calc_short_ssid */ > + rcu_read_lock(); > + ies = rcu_dereference(source_bss->ies); > + > + ap_info.tbtt_info_len = offsetofend(typeof(tbtt_info), > mld_params); > + ap_info.tbtt_info_hdr = > + u8_encode_bits(IEEE80211_TBTT_INFO_TYPE_TBTT, > + > IEEE80211_AP_INFO_TBTT_HDR_TYPE) | > + u8_encode_bits(0, > IEEE80211_AP_INFO_TBTT_HDR_COUNT); > + > + ap_info.channel = ieee80211_frequency_to_channel(source_bss- > >channel->center_freq); > + > + /* operating class */ > + elem = > cfg80211_find_elem(WLAN_EID_SUPPORTED_REGULATORY_CLASSES, > + ies->data, ies->len); > + if (elem && elem->datalen >= 1) { > + ap_info.op_class = elem->data[0]; > + } else { > + struct cfg80211_chan_def chandef; > + > + /* The AP is not providing us with anything to work > with. So > + * make up a somewhat reasonable operating class, but > don't > + * bother with it too much as no one will ever use > the > + * information. > + */ > + cfg80211_chandef_create(&chandef, source_bss- > >channel, > + NL80211_CHAN_NO_HT); > + > + if (!ieee80211_chandef_to_operating_class(&chandef, > + > &ap_info.op_class)) > + goto out_unlock; > + } > + > + /* Just set TBTT offset and PSD 20 to invalid/unknown */ > + tbtt_info.tbtt_offset = 255; > + tbtt_info.psd_20 = IEEE80211_RNR_TBTT_PARAMS_PSD_RESERVED; > + > + memcpy(tbtt_info.bssid, source_bss->bssid, ETH_ALEN); > + if (cfg80211_calc_short_ssid(ies, &elem, &short_ssid)) > + goto out_unlock; > + > + rcu_read_unlock(); > + > + tbtt_info.short_ssid = cpu_to_le32(short_ssid); > + > + tbtt_info.bss_params = IEEE80211_RNR_TBTT_PARAMS_SAME_SSID; > + > + if (is_mbssid) { > + tbtt_info.bss_params |= > IEEE80211_RNR_TBTT_PARAMS_MULTI_BSSID; > + tbtt_info.bss_params |= > IEEE80211_RNR_TBTT_PARAMS_TRANSMITTED_BSSID; > + } > + > + tbtt_info.mld_params.mld_id = 0; > + tbtt_info.mld_params.params = > + le16_encode_bits(link_id, > IEEE80211_RNR_MLD_PARAMS_LINK_ID) | > + le16_encode_bits(bss_change_count, > + > IEEE80211_RNR_MLD_PARAMS_BSS_CHANGE_COUNT); > + > + res = kzalloc(struct_size(res, data, > + sizeof(ap_info) + > ap_info.tbtt_info_len), > + gfp); > + if (!res) > + return NULL; > + > + /* Copy the data */ > + res->id = WLAN_EID_REDUCED_NEIGHBOR_REPORT; > + res->datalen = sizeof(ap_info) + ap_info.tbtt_info_len; > + memcpy(res->data, &ap_info, sizeof(ap_info)); > + memcpy(res->data + sizeof(ap_info), &tbtt_info, > ap_info.tbtt_info_len); > + > + return res; > + > +out_unlock: > + rcu_read_unlock(); > + return NULL; > +} > + > static void > cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, > struct > cfg80211_inform_single_bss_data *tx_data, > @@ -2627,13 +2724,14 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy > *wiphy, > .source_bss = source_bss, > .bss_source = BSS_SOURCE_STA_PROFILE, > }; > + struct element *reporter_rnr = NULL; > struct ieee80211_multi_link_elem *ml_elem; > struct cfg80211_mle *mle; > u16 control; > u8 ml_common_len; > - u8 *new_ie; > + u8 *new_ie = NULL; > struct cfg80211_bss *bss; > - int mld_id; > + u8 mld_id, reporter_link_id, bss_change_count; > u16 seen_links = 0; > const u8 *pos; > u8 i; > @@ -2655,8 +2753,14 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy > *wiphy, > > ml_common_len = ml_elem->variable[0]; > > - /* length + MLD MAC address + link ID info + BSS Params > Change Count */ > - pos = ml_elem->variable + 1 + 6 + 1 + 1; > + /* length + MLD MAC address */ > + pos = ml_elem->variable + 1 + 6; > + > + reporter_link_id = pos[0]; > + pos += 1; > + > + bss_change_count = pos[0]; > + pos += 1; > > if (u16_get_bits(control, > IEEE80211_MLC_BASIC_PRES_MED_SYNC_DELAY)) > pos += 2; > @@ -2687,10 +2791,21 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy > *wiphy, > if (!mle) > return; > > + /* No point in doing anything if there is no per-STA profile > */ > + if (!mle->sta_prof[0]) > + goto out; > + > new_ie = kmalloc(IEEE80211_MAX_DATA_LEN, gfp); > if (!new_ie) > goto out; > > + reporter_rnr = cfg80211_gen_reporter_rnr(source_bss, > + > u16_get_bits(control, > + > IEEE80211_MLC_BASIC_PRES_MLD_ID), > + mld_id == 0, > reporter_link_id, > + bss_change_count, > + gfp); > + > for (i = 0; i < ARRAY_SIZE(mle->sta_prof) && mle- > >sta_prof[i]; i++) { > const struct ieee80211_neighbor_ap_info *ap_info; > enum nl80211_band band; > @@ -2800,7 +2915,15 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy > *wiphy, > > data.ielen += sizeof(*ml_elem) + ml_common_len; > > - /* TODO: Add an RNR containing only the reporting AP > */ > + if (reporter_rnr && (use_for & > NL80211_BSS_USE_FOR_NORMAL)) { > + if (data.ielen + 1 + reporter_rnr->datalen > > + IEEE80211_MAX_DATA_LEN) > + continue; > + > + memcpy(new_ie + data.ielen, reporter_rnr, > + 1 + reporter_rnr->datalen); > + data.ielen += 1 + reporter_rnr->datalen; i.e. everywhere here it needs to add 2 (1 byte EID, 1 byte datalen). > + } > > bss = cfg80211_inform_single_bss_data(wiphy, &data, > gfp); > if (!bss) > @@ -2809,6 +2932,7 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy > *wiphy, > } > > out: > + kfree(reporter_rnr); > kfree(new_ie); > kfree(mle); > } Benjamin
diff --git a/net/wireless/scan.c b/net/wireless/scan.c index 42555753b947..dbb5885d40e7 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -2614,6 +2614,103 @@ cfg80211_tbtt_info_for_mld_ap(const u8 *ie, size_t ielen, u8 mld_id, u8 link_id, return 0; } +static struct element * +cfg80211_gen_reporter_rnr(struct cfg80211_bss *source_bss, bool is_mbssid, + bool same_mld, u8 link_id, u8 bss_change_count, + gfp_t gfp) +{ + const struct cfg80211_bss_ies *ies; + struct ieee80211_neighbor_ap_info ap_info; + struct ieee80211_tbtt_info_ge_11 tbtt_info; + u32 short_ssid; + const struct element *elem; + struct element *res; + + /* + * We only generate the RNR to permit ML lookups. For that we do not + * need an entry for the corresponding transmitting BSS, lets just skip + * it even though it would be easy to add. + */ + if (!same_mld) + return NULL; + + /* We could use tx_data->ies if we change cfg80211_calc_short_ssid */ + rcu_read_lock(); + ies = rcu_dereference(source_bss->ies); + + ap_info.tbtt_info_len = offsetofend(typeof(tbtt_info), mld_params); + ap_info.tbtt_info_hdr = + u8_encode_bits(IEEE80211_TBTT_INFO_TYPE_TBTT, + IEEE80211_AP_INFO_TBTT_HDR_TYPE) | + u8_encode_bits(0, IEEE80211_AP_INFO_TBTT_HDR_COUNT); + + ap_info.channel = ieee80211_frequency_to_channel(source_bss->channel->center_freq); + + /* operating class */ + elem = cfg80211_find_elem(WLAN_EID_SUPPORTED_REGULATORY_CLASSES, + ies->data, ies->len); + if (elem && elem->datalen >= 1) { + ap_info.op_class = elem->data[0]; + } else { + struct cfg80211_chan_def chandef; + + /* The AP is not providing us with anything to work with. So + * make up a somewhat reasonable operating class, but don't + * bother with it too much as no one will ever use the + * information. + */ + cfg80211_chandef_create(&chandef, source_bss->channel, + NL80211_CHAN_NO_HT); + + if (!ieee80211_chandef_to_operating_class(&chandef, + &ap_info.op_class)) + goto out_unlock; + } + + /* Just set TBTT offset and PSD 20 to invalid/unknown */ + tbtt_info.tbtt_offset = 255; + tbtt_info.psd_20 = IEEE80211_RNR_TBTT_PARAMS_PSD_RESERVED; + + memcpy(tbtt_info.bssid, source_bss->bssid, ETH_ALEN); + if (cfg80211_calc_short_ssid(ies, &elem, &short_ssid)) + goto out_unlock; + + rcu_read_unlock(); + + tbtt_info.short_ssid = cpu_to_le32(short_ssid); + + tbtt_info.bss_params = IEEE80211_RNR_TBTT_PARAMS_SAME_SSID; + + if (is_mbssid) { + tbtt_info.bss_params |= IEEE80211_RNR_TBTT_PARAMS_MULTI_BSSID; + tbtt_info.bss_params |= IEEE80211_RNR_TBTT_PARAMS_TRANSMITTED_BSSID; + } + + tbtt_info.mld_params.mld_id = 0; + tbtt_info.mld_params.params = + le16_encode_bits(link_id, IEEE80211_RNR_MLD_PARAMS_LINK_ID) | + le16_encode_bits(bss_change_count, + IEEE80211_RNR_MLD_PARAMS_BSS_CHANGE_COUNT); + + res = kzalloc(struct_size(res, data, + sizeof(ap_info) + ap_info.tbtt_info_len), + gfp); + if (!res) + return NULL; + + /* Copy the data */ + res->id = WLAN_EID_REDUCED_NEIGHBOR_REPORT; + res->datalen = sizeof(ap_info) + ap_info.tbtt_info_len; + memcpy(res->data, &ap_info, sizeof(ap_info)); + memcpy(res->data + sizeof(ap_info), &tbtt_info, ap_info.tbtt_info_len); + + return res; + +out_unlock: + rcu_read_unlock(); + return NULL; +} + static void cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, struct cfg80211_inform_single_bss_data *tx_data, @@ -2627,13 +2724,14 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, .source_bss = source_bss, .bss_source = BSS_SOURCE_STA_PROFILE, }; + struct element *reporter_rnr = NULL; struct ieee80211_multi_link_elem *ml_elem; struct cfg80211_mle *mle; u16 control; u8 ml_common_len; - u8 *new_ie; + u8 *new_ie = NULL; struct cfg80211_bss *bss; - int mld_id; + u8 mld_id, reporter_link_id, bss_change_count; u16 seen_links = 0; const u8 *pos; u8 i; @@ -2655,8 +2753,14 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, ml_common_len = ml_elem->variable[0]; - /* length + MLD MAC address + link ID info + BSS Params Change Count */ - pos = ml_elem->variable + 1 + 6 + 1 + 1; + /* length + MLD MAC address */ + pos = ml_elem->variable + 1 + 6; + + reporter_link_id = pos[0]; + pos += 1; + + bss_change_count = pos[0]; + pos += 1; if (u16_get_bits(control, IEEE80211_MLC_BASIC_PRES_MED_SYNC_DELAY)) pos += 2; @@ -2687,10 +2791,21 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, if (!mle) return; + /* No point in doing anything if there is no per-STA profile */ + if (!mle->sta_prof[0]) + goto out; + new_ie = kmalloc(IEEE80211_MAX_DATA_LEN, gfp); if (!new_ie) goto out; + reporter_rnr = cfg80211_gen_reporter_rnr(source_bss, + u16_get_bits(control, + IEEE80211_MLC_BASIC_PRES_MLD_ID), + mld_id == 0, reporter_link_id, + bss_change_count, + gfp); + for (i = 0; i < ARRAY_SIZE(mle->sta_prof) && mle->sta_prof[i]; i++) { const struct ieee80211_neighbor_ap_info *ap_info; enum nl80211_band band; @@ -2800,7 +2915,15 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, data.ielen += sizeof(*ml_elem) + ml_common_len; - /* TODO: Add an RNR containing only the reporting AP */ + if (reporter_rnr && (use_for & NL80211_BSS_USE_FOR_NORMAL)) { + if (data.ielen + 1 + reporter_rnr->datalen > + IEEE80211_MAX_DATA_LEN) + continue; + + memcpy(new_ie + data.ielen, reporter_rnr, + 1 + reporter_rnr->datalen); + data.ielen += 1 + reporter_rnr->datalen; + } bss = cfg80211_inform_single_bss_data(wiphy, &data, gfp); if (!bss) @@ -2809,6 +2932,7 @@ cfg80211_parse_ml_elem_sta_data(struct wiphy *wiphy, } out: + kfree(reporter_rnr); kfree(new_ie); kfree(mle); }