[OpenWrt-Devel] [PATCH] iwinfo: do not wait for scan results if scan request failed.

[Dmitrijs Ivanovs]

Hi John!

1) The code is reachable because "err" is signed integer. Negative
value is error indication.
2) Cleanup is called later in all cases. Actually, calling it here
leads to an error - a kind of "double free". It was never encountered
because it was never called in past.

I had to create this patch after transition to the new kernel because
AP scan patch was accidentally removed. Inability of kernel to do wifi
scan in AP mode caused this situation where libiwinfo had to wait
indefinitely for scan results. Ok, AP scan patch is restored now, but
libiwinfo should be more robust against similar situations in future.
In fact, I have tested my patch with both AP scan turned on and off.


On Mon, Aug 17, 2015 at 1:18 PM, John Crispin <blogic at openwrt.org> wrote:
> Hi,
>
> I have various issues with this patch
>
> On 31/07/2015 10:53, Dmitry Ivanov wrote:
>> Do not wait for scan results if scan request failed.
>>
>> Signed-off-by: Dmitry Ivanov <dima at ubnt.com>
>> ---
>>  iwinfo_nl80211.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
>> index 900eef2..251ec33 100644
>> --- a/iwinfo_nl80211.c
>> +++ b/iwinfo_nl80211.c
>> @@ -389,11 +389,12 @@ static struct nl80211_msg_conveyor * nl80211_send(
>>       while (err > 0)
>>               nl_recvmsgs(nls->nl_sock, cv->cb);
>>
>> +     if (err)
>> +             goto err;
>> +
>
> this code will never be reached due tot he while (err > 0) above
>
>
>>       return &rcv;
>>
>>  err:
>> -     nl_cb_put(cv->cb);
>> -     nlmsg_free(cv->msg);
>>
>
> removing the freeeing code from a global helper is not a good idea and
> will cause a leak.
>
>
>         John
>
>
>
>>       return NULL;
>>  }
>> @@ -2016,16 +2017,21 @@ static int nl80211_get_scanlist_cb(struct nl_msg *msg, void *arg)
>>
>>  static int nl80211_get_scanlist_nl(const char *ifname, char *buf, int *len)
>>  {
>> -     struct nl80211_msg_conveyor *req;
>> +     struct nl80211_msg_conveyor *req, *scan_res = NULL;
>>       struct nl80211_scanlist sl = { .e = (struct iwinfo_scanlist_entry *)buf };
>>
>>       req = nl80211_msg(ifname, NL80211_CMD_TRIGGER_SCAN, 0);
>>       if (req)
>>       {
>> -             nl80211_send(req, NULL, NULL);
>> +             scan_res = nl80211_send(req, NULL, NULL);
>>               nl80211_free(req);
>>       }
>>
>> +     if (!scan_res)
>> +     {
>> +             return -1;
>> +     }
>> +
>>       nl80211_wait("nl80211", "scan", NL80211_CMD_NEW_SCAN_RESULTS);
>>
>>       req = nl80211_msg(ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP);
>>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel