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

Submitted by Dmitry Shmidt on Nov. 12, 2013, 8:41 p.m.

Details

Message ID 20131112204358.74FC913FECA@ushik.mtv.corp.google.com
State Changes Requested
Headers show

Commit Message

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(+)

Comments

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 hide | download patch | download mbox

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);
 }