Re: wpa_supplicant 2.10 - Scan failed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Etienne,

On 1/30/22 00:25, Étienne Morice wrote:
Update as I've been doing a few more checks.

I should specify these tests are with the broadcom-wl 6.30.223.271-357
driver on a Broadcom BCM43142 controller.

First, I checked why the new SCS/MSCS was making the scan fail.

The reason is, it causes nl80211_scan_common in
src/drivers/driver_nl80211_scan.c to add some extra IEs in the scan
request, via

As a sidenote: Similar breakage was observed a while back with broadcom-wl
when Debian and Arch enabled CONFIG_WNM for their wpa_supplicant
distributions [0].

The kernel exposes the maximum supported bytes to userspace with
which can be retrieved in wiphy_info_handler.

See the attached patch. Does scanning work with thid patch applied to v2.10?

[0] https://bugs.archlinux.org/task/65588

Best
David


 > nla_put(msg, NL80211_ATTR_IE, params->extra_ies_len, params->extra_ies)

(commenting out this block also makes the scans succeed again)

and these extra_ies cause the cfg80211 kernel module to reject the scan
request with EINVAL based on the following check in
linux/net/wireless/nl80211.c in nl80211_trigger_scan :

 >  if (ie_len > wiphy->max_scan_ie_len)
 > return -EINVAL;

In this case, ie_len is set to 13 in the scan request and
max_scan_ie_len is set (default initialized) to 0 in the wl driver,
hence the reported behavior.

To double check I recompiled my broadcom-wl driver with this added
initialization line:

 > wdev->wiphy->max_scan_ie_len = 1024;

(meaning the driver still completely ignores the extra IEs, but it
declares that it accepts some)

and indeed the scan also started to work again with the original 2.10
release.

So my takeaway is that one of these two should happen:
  1) wpa_supplicant should check if the driver accepts extra IEs in the
scan request before actually putting any.
  2) drivers should be patched to make sure they accept extra IEs in the
scan request even if they then silently discard them.

Then I'm not qualified to say which of these two (or something else
entirely) is the proper fix. However the fact that the cfg80211 module
has an explicit check for it makes me guess that drivers are allowed to
limit extra IEs and that wpa_supplicant should honor this limit ?

Best,
Étienne Morice

On 1/27/22 13:06, Étienne Morice wrote:
Hi,

Following the update from 2.9 to 2.10 in arch, we had some scan failures
reported.

Original report is here:

https://bugs.archlinux.org/task/73495

Upon updating and restarting wpa_supplicant, scan starts to fail with a
log of this kind:

systemd[1]: Started WPA supplicant daemon (interface-specific version).
Jan 26 22:33:45 ... wpa_supplicant[422]: Successfully initialized
wpa_supplicant
Jan 26 22:33:45 ... wpa_supplicant[422]: wlp3s0:
CTRL-EVENT-SCAN-FAILED ret=-22 retry=1
Jan 26 22:33:46 ... wpa_supplicant[422]: wlp3s0:
CTRL-EVENT-SCAN-FAILED ret=-22 retry=1

In one case at least bisecting the problem was possible, here are the
conclusions pasted from the arch thread:

=====================================

For me the first commits that make it stop working are:
a11804724 MSCS: Add support to send MSCS Request frames
c005283c4 SCS: Sending of SCS Request frames

These introduced in wpa_supplicant/wpa_supplicant.c, in
wpas_ext_capab_byte, two new flags:
```
case 6: /* Bits 48-55 */
         *pos |= 0x40; /* Bit 54 - SCS */
```
and
```
case 10: /* Bits 80-87 */
         *pos |= 0x20; /* Bit 85 - Mirrored SCS */
```

If I comment these out, or set mscs = scs = false at the beginning of
the function:
```
diff --git a/wpa_supplicant/wpa_supplicant.c
b/wpa_supplicant/wpa_supplicant.c
index d37a994f9..561e5f3ed 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -1883,7 +1883,7 @@ int wpa_supplicant_set_suites(struct
wpa_supplicant *wpa_s,

  static void wpas_ext_capab_byte(struct wpa_supplicant *wpa_s, u8 *pos,
int idx)
  {
-       bool scs = true, mscs = true;
+       bool scs = false, mscs = false;

         *pos = 0x00;
```
I get it to work again. I'm posting from hostap_2_10 with this
modification right now.

Also, commenting the rest of the first breaking commit made no
difference, so it seems to me that it's really these flags that cause
the problem for me and not the rest of code depending on them introduced
at the same time.

Now the problem is, I have no idea what these actually do and how they
cause the scan to fail.

Most crucially, I don't know at all if wpa_supplicant is doing something
wrong here, or if that code is actually correct but these flags are
somehow propagated down the stack to components (drivers ?) that cannot
handle them yet. So I'm not even sure if this should be fixed upstream
here or in other packages.

======================================

Any idea how to proceed from here ?

Best,

Étienne Morice



_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
From 05a5ff7abec6fb4cd16273b9dc90776ad2385f38 Mon Sep 17 00:00:00 2001
From: David Bauer <mail@xxxxxxxxxxxxxxx>
Date: Sun, 30 Jan 2022 13:44:16 +0100
Subject: [PATCH] nl80211: add extra-ies only if allowed by driver

Signed-off-by: David Bauer <mail@xxxxxxxxxxxxxxx>
---
 src/drivers/driver.h              | 3 +++
 src/drivers/driver_nl80211_capa.c | 4 ++++
 src/drivers/driver_nl80211_scan.c | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)

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..62d63cd5b 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,
-- 
2.35.0

_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap

[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux