| Message ID | 20220130192200.10883-1-mail@david-bauer.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | nl80211: add extra-ies only if allowed by driver | expand |
On Sun, 2022-01-30 at 20:22 +0100, David Bauer wrote: > > - if (params->extra_ies) { > + if (params->extra_ies && drv->capa.max_scan_ie_len >= params->extra_ies_len) { > wpa_hexdump(MSG_MSGDUMP, "nl80211: Scan extra IEs", > params->extra_ies, params->extra_ies_len); I guess that makes sense if capa.max_scan_ie_len is zero, but if not perhaps it'd be nice to be able to omit _some_ of the stuff? Also, higher layers of the supplicant might expect their elements to be added, so I'm not sure what to make of this, that's a bit annoying. Perhaps some configurations shouldn't be allowed if the capa.max_scan_ie_len is really small? johannes
Hi Johannes, On 1/30/22 21:14, Johannes Berg wrote: > On Sun, 2022-01-30 at 20:22 +0100, David Bauer wrote: >> >> - if (params->extra_ies) { >> + if (params->extra_ies && drv->capa.max_scan_ie_len >= params->extra_ies_len) { >> wpa_hexdump(MSG_MSGDUMP, "nl80211: Scan extra IEs", >> params->extra_ies, params->extra_ies_len); > > I guess that makes sense if capa.max_scan_ie_len is zero, but if not > perhaps it'd be nice to be able to omit _some_ of the stuff? > > Also, higher layers of the supplicant might expect their elements to be > added, so I'm not sure what to make of this, that's a bit annoying. > > Perhaps some configurations shouldn't be allowed if the > capa.max_scan_ie_len is really small? Good points. I'm not sure how we want this to be handled. All extra-IEs are added in wpa_supplicant_extra_ies and ultimately it is here where the resulting buffer size is determined. We could prioritize there and add extra-IEs ordered until the buffer is full. Tracing this down to hostapd initializing the interface however is very complex, so not sure if this road is worth the effort. That being said, we only touch the IEs present in probe requests, so the impact on the resulting link should not be of an issue. What would be your approach on that? Best David > > johannes
On Sun, Jan 30, 2022 at 08:22:00PM +0100, David Bauer wrote: > Upgrading wpa_supplicant from 2.9 to 2.10 breaks broadcom-wl > based adapters. The reason for it is hostapd tries to install additional > IEs for scanning while the driver does not support this. Can you please be a bit more specific on what you mean with "breaking" in this context? What exactly is broken? Just active scanning (i.e., sending the Probe Request frames)? Or scanning in general (i.e., not getting any scan results at all)? Or something else? For most cases, it is likely fine to omit extra IEs from Probe Request frames, but this does break some cases like WPS PBC and P2P.
Hi Jouni, On 1/31/22 13:08, Jouni Malinen wrote: > On Sun, Jan 30, 2022 at 08:22:00PM +0100, David Bauer wrote: >> Upgrading wpa_supplicant from 2.9 to 2.10 breaks broadcom-wl >> based adapters. The reason for it is hostapd tries to install additional >> IEs for scanning while the driver does not support this. > > Can you please be a bit more specific on what you mean with "breaking" > in this context? What exactly is broken? Just active scanning (i.e., > sending the Probe Request frames)? Or scanning in general (i.e., not > getting any scan results at all)? Or something else? See the thread "wpa_supplicant 2.10 - Scan failed" from 27.01.22. The problem is sending the probe requests for active scanning. > > For most cases, it is likely fine to omit extra IEs from Probe Request > frames, but this does break some cases like WPS PBC and P2P. > Didn't think of those - Would you suggest we assign a priority for those IEs when crafting the extra-IE buffer? We could handle those IEs as mandatory (and fail in this stage) while adding other IEs optionally. Best David
diff --git a/src/drivers/driver.h b/src/drivers/driver.h index d3312a34d..b5b626451 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -2052,6 +2052,9 @@ struct wpa_driver_capa { /** Maximum number of iterations in a single scan plan */ u32 max_sched_scan_plan_iterations; + /** Maximum number of extra IE bytes for scans */ + u16 max_scan_ie_len; + /** Whether sched_scan (offloaded scanning) is supported */ int sched_scan_supported; diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c index 83868b78e..b33b6badb 100644 --- a/src/drivers/driver_nl80211_capa.c +++ b/src/drivers/driver_nl80211_capa.c @@ -885,6 +885,10 @@ static int wiphy_info_handler(struct nl_msg *msg, void *arg) nla_get_u32(tb[NL80211_ATTR_MAX_SCAN_PLAN_ITERATIONS]); } + if (tb[NL80211_ATTR_MAX_SCAN_IE_LEN]) + capa->max_scan_ie_len = + nla_get_u16(tb[NL80211_ATTR_MAX_SCAN_IE_LEN]); + if (tb[NL80211_ATTR_MAX_MATCH_SETS]) capa->max_match_sets = nla_get_u8(tb[NL80211_ATTR_MAX_MATCH_SETS]); diff --git a/src/drivers/driver_nl80211_scan.c b/src/drivers/driver_nl80211_scan.c index 131608480..b0f095192 100644 --- a/src/drivers/driver_nl80211_scan.c +++ b/src/drivers/driver_nl80211_scan.c @@ -207,7 +207,7 @@ nl80211_scan_common(struct i802_bss *bss, u8 cmd, wpa_printf(MSG_DEBUG, "nl80211: Passive scan requested"); } - if (params->extra_ies) { + if (params->extra_ies && drv->capa.max_scan_ie_len >= params->extra_ies_len) { wpa_hexdump(MSG_MSGDUMP, "nl80211: Scan extra IEs", params->extra_ies, params->extra_ies_len); if (nla_put(msg, NL80211_ATTR_IE, params->extra_ies_len,