diff mbox

RRM: Fix number of SSIDs in scan for beacon report

Message ID 1488963277-11480-1-git-send-email-andrei.otcheretianski@intel.com
State Changes Requested
Headers show

Commit Message

Andrei Otcheretianski March 8, 2017, 8:54 a.m. UTC 
From: Avraham Stern <avraham.stern@intel.com>

When requesting a scan for beacon report request, the number of
SSIDs is always set to one, even when no SSID subelement is
present in the measurement request.

Fix this by setting the SSID in the scan parameters only if the
SSID subelement is actually present in the measurement request.

Signed-off-by: Avraham Stern <avraham.stern@intel.com>
---
 wpa_supplicant/rrm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jouni Malinen March 8, 2017, 2:01 p.m. UTC | #1 
On Wed, Mar 08, 2017 at 10:54:37AM +0200, Andrei Otcheretianski wrote:
> When requesting a scan for beacon report request, the number of
> SSIDs is always set to one, even when no SSID subelement is
> present in the measurement request.

Only when Measurement Mode is set to Active; not always (i.e.,
Measurement Mode set to Passive would not do this)..

> Fix this by setting the SSID in the scan parameters only if the
> SSID subelement is actually present in the measurement request.

What is the incorrect behavior that this is fixing?

> diff --git a/wpa_supplicant/rrm.c b/wpa_supplicant/rrm.c
> @@ -1085,7 +1085,7 @@ wpas_rm_handle_beacon_req(struct wpa_supplicant *wpa_s,
>  
>  	params->only_new_results = 1;
>  
> -	if (req->mode == BEACON_REPORT_MODE_ACTIVE) {
> +	if (req->mode == BEACON_REPORT_MODE_ACTIVE && data->ssid_len) {
>  		params->ssids[params->num_ssids].ssid = data->ssid;
>  		params->ssids[params->num_ssids++].ssid_len = data->ssid_len;
>  	}

This is a beacon report request with Measurement Mode having value
Active. That indicates that there is a request to do an active scan. If
the SSID subelement is not included in the Beacon request, this is a
request to report all BSSs regardless of the SSID. For this to use an
active scan, params->num_ssids needs to be 1 and a zero length SSID
needs to be specified as the SSID to scan for.

The proposed change seems to break the expected behavior for Measurement
Mode = Active. As such, I'm not applying this without more thorough
commit message explaining why the current behavior is incorrect
(ideally, point to the IEEE 802.11 standard clause describing this; IEEE
Std 802.11-2016, 11.11.9.1 (Beacon report) seems to match the current
implementation behavior).
Andrei Otcheretianski March 8, 2017, 3:36 p.m. UTC | #2 
> This is a beacon report request with Measurement Mode having value Active.
> That indicates that there is a request to do an active scan. If the SSID subelement
> is not included in the Beacon request, this is a request to report all BSSs
> regardless of the SSID. For this to use an active scan, params->num_ssids needs
> to be 1 and a zero length SSID needs to be specified as the SSID to scan for.

Please drop this one. I somehow missed the num_ssid == 1 for active scan part.

> Jouni Malinen                                            PGP id EFC895FA
diff mbox

Patch

diff --git a/wpa_supplicant/rrm.c b/wpa_supplicant/rrm.c
index 5be917c..f4e7360 100644
--- a/wpa_supplicant/rrm.c
+++ b/wpa_supplicant/rrm.c
@@ -1085,7 +1085,7 @@  wpas_rm_handle_beacon_req(struct wpa_supplicant *wpa_s,
 
 	params->only_new_results = 1;
 
-	if (req->mode == BEACON_REPORT_MODE_ACTIVE) {
+	if (req->mode == BEACON_REPORT_MODE_ACTIVE && data->ssid_len) {
 		params->ssids[params->num_ssids].ssid = data->ssid;
 		params->ssids[params->num_ssids++].ssid_len = data->ssid_len;
 	}