Patchwork [2/2] Ensure reset of wpa_s->scanning if scan is cancelled

login
register
mail settings
Submitter Dmitry Shmidt
Date Nov. 12, 2013, 8:41 p.m.
Message ID <20131112204358.74FC913FECA@ushik.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/290774/
State Changes Requested
Headers show

Comments

Dmitry Shmidt - Nov. 12, 2013, 8:41 p.m.
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
---
 wpa_supplicant/scan.c | 1 +
 1 file changed, 1 insertion(+)
Jouni Malinen - Nov. 24, 2013, 10:14 a.m.
On Tue, Nov 12, 2013 at 12:41:58PM -0800, Dmitry Shmidt wrote:
> diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
> @@ -1217,6 +1217,7 @@ void wpa_supplicant_cancel_scan(struct wpa_supplicant *wpa_s)
>  	wpa_dbg(wpa_s, MSG_DEBUG, "Cancelling scan request");
>  	eloop_cancel_timeout(wpa_supplicant_scan, wpa_s, NULL);
>  	wpas_p2p_continue_after_scan(wpa_s);
> +	wpa_supplicant_notify_scanning(wpa_s, 0);

This does not look correct and can result in unexpected notifications of
scanning being stopped. wpa_supplicant_cancel_scan() cancels a scheduled
scan request, not an ongoing scan. If there is no ongoing scan,
wpa_s->scanning == 0 and this change would not achieve anything. If
there is an ongoing scan, this would claim that the scan has been
completed, even though it has not, and as such, this could result in
incorrect behavior.

Why would this change be needed and is there really justification for
breaking the existing scan notification mechanism because of that?
Dmitry Shmidt - Nov. 24, 2013, 7:35 p.m.
On Sun, Nov 24, 2013 at 2:14 AM, Jouni Malinen <j@w1.fi> wrote:
> On Tue, Nov 12, 2013 at 12:41:58PM -0800, Dmitry Shmidt wrote:
>> diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
>> @@ -1217,6 +1217,7 @@ void wpa_supplicant_cancel_scan(struct wpa_supplicant *wpa_s)
>>       wpa_dbg(wpa_s, MSG_DEBUG, "Cancelling scan request");
>>       eloop_cancel_timeout(wpa_supplicant_scan, wpa_s, NULL);
>>       wpas_p2p_continue_after_scan(wpa_s);
>> +     wpa_supplicant_notify_scanning(wpa_s, 0);
>
> This does not look correct and can result in unexpected notifications of
> scanning being stopped. wpa_supplicant_cancel_scan() cancels a scheduled
> scan request, not an ongoing scan. If there is no ongoing scan,
> wpa_s->scanning == 0 and this change would not achieve anything. If
> there is an ongoing scan, this would claim that the scan has been
> completed, even though it has not, and as such, this could result in
> incorrect behavior.

Makes sense. Indeed, code is obsolete. Thanks.

>
> Why would this change be needed and is there really justification for
> breaking the existing scan notification mechanism because of that?
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap

Patch

diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index 96c127b..41e0dfc 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -1217,6 +1217,7 @@  void wpa_supplicant_cancel_scan(struct wpa_supplicant *wpa_s)
 	wpa_dbg(wpa_s, MSG_DEBUG, "Cancelling scan request");
 	eloop_cancel_timeout(wpa_supplicant_scan, wpa_s, NULL);
 	wpas_p2p_continue_after_scan(wpa_s);
+	wpa_supplicant_notify_scanning(wpa_s, 0);
 }