Found using lsan:
==29896==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 9 byte(s) in 1 object(s) allocated from:
#0 0x7fcd41e0c710 in __interceptor_malloc /var/tmp/portage/sys-devel/gcc-8.2.0-r6/work/gcc-8.2.0/libsanitizer/asan/asan_malloc_linux.cc:86
#1 0x606abd in l_malloc ell/util.c:62
#2 0x460230 in ie_tlv_vendor_ie_concat src/ie.c:140
#3 0x4605d1 in ie_tlv_extract_wfd_payload src/ie.c:216
#4 0x4a8773 in scan_parse_bss_information_elements src/scan.c:1105
#5 0x4a94a8 in scan_parse_attr_bss src/scan.c:1181
#6 0x4a99f8 in scan_parse_result src/scan.c:1238
#7 0x4abe4e in get_scan_callback src/scan.c:1451
#8 0x6442d9 in process_unicast ell/genl.c:979
#9 0x6453ff in received_data ell/genl.c:1087
#10 0x62e1a4 in io_callback ell/io.c:126
#11 0x628fca in l_main_iterate ell/main.c:473
#12 0x6294e8 in l_main_run ell/main.c:520
#13 0x629d8b in l_main_run_with_signal ell/main.c:642
#14 0x40681b in main src/main.c:505
#15 0x7fcd40a55bdd in __libc_start_main (/lib64/libc.so.6+0x21bdd)
Besides being undefined behaviour, signed integer overflow can cause
unexpected comparison results. In the case of network_rank_compare(),
a connected network with rank INT_MAX would cause newly inserted
networks with negative rank to be inserted earlier in the ordered
network list. This is reflected in the GetOrderedMethods() DBus method
as can be seen in the following iwctl output:
[iwd]# station wlan0 get-networks
Network name Security Signal
----------------------------------------------------
BEOLAN 8021x **** }
BeoBlue psk *** } all unknown,
UI_Test_Network psk *** } hence assigned
deneb_2G psk *** } negative rank
BEOGUEST open **** }
> titan psk ****
Linksys05274_5GHz_dmt psk ****
Lyngby-4G-4 5GHz psk ****
Instead of creating the results->bss_list l_queue lazily, always create
one before sending the GET_SCAN command. This is to make sure that an
empty list is passed to the scan callback (e.g. in station.c) instead of
a NULL. Passing NULL has been causing difficult to debug crashes in
station.c, in fact I think I've been seeing them for over a year now
but can't be sure. station_set_scan_results has been taking ownership
of the new BSS list and, if station->connected_bss was not on the list,
it would try to add it not realizing that l_queue_push_tail() was doing
nothing. Always passing a valid list may help us prevent similar
problems in the future.
The crash might start with:
==120489== Invalid read of size 8
==120489== at 0x425D38: network_bss_select (network.c:709)
==120489== by 0x415BD1: station_try_next_bss (station.c:2263)
==120489== by 0x415E31: station_retry_with_status (station.c:2323)
==120489== by 0x415E31: station_connect_cb (station.c:2367)
==120489== by 0x407E66: netdev_connect_failed (netdev.c:569)
==120489== by 0x40B93D: netdev_connect_event (netdev.c:1801)
==120489== by 0x40B93D: netdev_mlme_notify (netdev.c:3678)
To use the wiphy radio work queue, scanning mostly remained the same.
start_next_scan_request was modified to be used as the work callback,
as well as not start the next scan if the current one was done
(since this is taken care of by wiphy work queue now). All
calls to start_next_scan_request were removed, and more or less
replaced with wiphy_radio_work_done.
scan_{suspend,resume} were both removed since radio management
priorities solve this for us. ANQP requests can be inserted ahead of
scan requests, which accomplishes the same thing.
If start_scan_next_request() is called while a scan request
(NL80211_CMD_TRIGGER_SCAN) is still running, the same scan request will
be sent again. Add a check in the function to avoid sending a request if
one is already in progress. For consistency, check also that scan
results are not being requested (NL80211_CMD_GET_SCAN), before trying to
send the next scan request. Finally, remove similar checks at
start_next_scan_request() callsites to simplify the code.
This also fixes a crash that occurs if the following conditions are met:
- the duplicated request is the only request in the scan request
queue, and
- both scan requests fail with an error not EBUSY.
In this case, the first callback to scan_request_triggered() will delete
the request from the scan request queue. The second callback will find
an empty queue and consequently pass a NULL scan_request pointer to
scan_request_failed(), causing a segmentation fault.
If scanning is suspended, have scan_common() queue its scan request
rather than issuing it immediately. This respects the assumption that
scans are not requested while sc->suspended is true.
#0 0x000055555558ee5d in scan_notify (msg=0x55555560b640, user_data=0x0) at src/scan.c:1706
#1 0x00007ffff7f2c78c in ?? () from /usr/lib/libell.so.0
#2 0x00007ffff7f299ec in ?? () from /usr/lib/libell.so.0
#3 0x00007ffff7f28e4a in l_main_iterate () from /usr/lib/libell.so.0
#4 0x00007ffff7f28efc in l_main_run () from /usr/lib/libell.so.0
#5 0x00007ffff7f290b9 in l_main_run_with_signal () from /usr/lib/libell.so.0
#6 0x00005555555639c4 in main (argc=1, argv=0x7fffffffec18) at src/main.c:497
Save the source frame type in struct scan_bss as it may affect how some
of the data in the struct will be parsed. Also replace the P2P IE
payload data in that struct with a union containing pre-parsed p2p
attributes corresponding to the frame type.
This means users don't have to call the parsers in p2putil.c on that
data, which wouldn't have worked anyway because those parsers assume
input is the raw IE sequence rather than just the "payload".
The kernel sends NL80211_ATTR_SCAN_START_TIME_TSF with CMD_TRIGGER and
RRM requires this value for beacon measurement reports.
The start time is parsed during CMD_TRIGGER and set into the scan request.
A getter was added to obtain this time value for an already triggered
scan.
After making the change, the SCAN_ABORTED case was cleaned up a bit to
remove the local scan_request usage in favor of the one used for all the
other cases.
The kernel allows a scan duration and duration mandatory flag to be
set in scan requests. RRM requests can contain these values so they
have been added to scan_parameters.
Scanning with drivers which do not support EXT_FEATURE_SET_SCAN_DWELL
will not include these values in scan requests.
no_cck_rates is set in the scan parameters generally to make sure
that the Probe Request frames are not sent at any of the 802.11b
rates during active scans. With this patch we also omit those rates
from the Supported Rates IEs, which is required by the p2p spec and
also matches our flag's name.
The intent here was to validate that the frequency is a multiple of 5
and lies in a certain range. Somehow the channel was checked for being
a multiple of 5 instead.
This will be seen in Probe Requests. More IEs can and should
be added here depending on the support in IWD. E.g. HS20 indication,
Interworking, HT/VHT IE's etc.
For (Re)Association the HS20 indication element was passed exactly as
it was found in the scan results. The spec defines what bits can be
set and what cannot when this IE is used in (Re)Association. Instead
of assuming the AP's IE conforms to the spec, we now parse the IE and
re-build it for use with (Re)Association.
Since the full IE is no longer used, it was removed from scan_bss, and
replaced with a bit for HS20 support (hs20_capable). This member is
now used the same as hs20_ie was.
The version parsed during scan results is now used when building the
(Re)Association IE.
The HS20 indication element should always be included during
(Re)Association per the spec. This removes the need for a
dedicated boolean, and now the hs20_ie can be used instead.
If the scan was triggered and later aborted, make sure to reset the
triggered value when the CMD_NEW_SCAN_RESULTS event comes in.
src/station.c:station_enter_state() Old State: disconnected, new state: connecting
src/scan.c:scan_notify() Scan notification 33
src/station.c:station_netdev_event() Associating
src/scan.c:scan_notify() Scan notification 34
Aborting (signal 11) [/home/denkenz/iwd-master/src/iwd]
++++++++ backtrace ++++++++
#0 0x7efd4d6a2ef0 in /lib64/libc.so.6
#1 0x42b20d in scan_notify() at src/scan.c:1383
P2P probe requests are to be sent at min 6.0 Mb/s using OFDM,
specifically the 802.11b rates are prohibited (section 2.4.1 in Wi-Fi
P2p Technical Spec v1.7), some of which use CCK modulation. This is
already the default for 5G but for 2.4G the drivers generally do this
if we set the NL80211_ATTR_TX_NO_CCK_RATE flags with
NL80211_CMD_TRIGGER_SCAN.
The ifindex is used to index the netdevs in the system (wlan, ethernet,
etc.) but we can also do wifi scanning on interfaces that have no
corresponding netdev object, like the P2P-device virtual interfaces.
Use the wdev id's to reference interfaces, the nl80211 api doesn't care
whether we use a NL80211_ATTR_IFINDEX or NL80211_ATTR_WDEV. Only
wireless interfaces have a wdev id.
Save the actual cmd_id returned from l_genl_family_dump and zero it in
the get_scan_done. There's no need to zero it in scan_cancel because
get_scan_done gets called automatically.
Store the scan_context pointer in scan_results directly instead of
storing the ifindex. We now cancel ongoing GET_SCAN commands when the
scan_context is being freed so there's no point going through the extra
step of looking up the scan_context by ifindex inside the command
callback to guard against non-existent scan_contexts.
In order to do ANQP efficiently IWD needs the ability to suspend scanning
temporarily. This is because both scanning and ANQP go offchannel and must
remain off channel for some amount of time. This cannot be done
simultaneously and if e.g. ANQP is requested after a scan is already
pending, the kernel will wait till that scan finishes before sending out
the frame.
