Update station->networks_sorted with new networks added inside station_hidden_network_scan_results() because we rely on station->networks being in sync with station->networks_sorted. Otherwise station_set_scan_results may crash. Optimally a generic scan shouldn't happen while we're connecting to a new hidden network because the generic scan doesn't know about the hidden network's SSID and will not include the SSID in the probe request so it can at most expire the BSS we're trying to connect to. However we can't easily tell whether we're in the middle of a Connect() or ConnectHiddenNetwork() call as mentioned in the previous commit, so we have to accept that we may trigger a new scan while waiting for secrets from the agent. Thus the BSS discovered in the active scan for the hidden SSID may expire if the agent request takes too long but we this change at least we don't crash. --- src/station.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/station.c b/src/station.c index 6ac9e53b..b70d3416 100644 --- a/src/station.c +++ b/src/station.c @@ -2572,11 +2572,16 @@ static bool station_hidden_network_scan_results(int err, memcmp(bss->ssid, ssid, ssid_len)) goto next; - if (station_add_seen_bss(station, bss)) { - l_queue_push_tail(station->bss_list, bss); + network = station_add_seen_bss(station, bss); + if (!network) + goto next; - continue; - } + l_queue_push_tail(station->bss_list, bss); + network_rank_update(network, false); + l_queue_remove(station->networks_sorted, network); + l_queue_insert(station->networks_sorted, network, + network_rank_compare, NULL); + continue; next: scan_bss_free(bss); -- 2.25.1

Hi Denis, On Thu, 8 Oct 2020 at 17:19, Denis Kenzior <denkenz(a)gmail.com&gt; wrote: I was running under valgrind and it just reported invalid accesses but didn't crash that time. Right, so we don't call network_bss_list_clear() on the hidden network but we do free the old BSS because the new bss list has the same BSS in it (in my case it did include the SSID, it was identical). Next we try to insert the new scan_bss into the network and scan_bss_rank_compare() triggers an invalid access. True, in that case I guess the old BSS gets freed and the network gets removed. We probably need to treat it as a special case in station_set_scan_results. Inside station_set_scan_results? That would work too, but I think it may be better to be consistent and report this network like other networks in case it confuses clients. Right, I didn't realize the whole network would get removed. If it was just the BSSes then the next time network.c tries to select a BSS it would just abort connection, but if the network goes away we crash. So I didn't bother fixing every scenario in this patchset. I was thinking ConnectHiddenNetwork should set connected_bss, connected_network from the beginning, and station_is_busy() should report true. network.c and wsc.c should look at station_is_busy() before accepting a connection, and should also notify station that they have a pending Connect() / PushButton() / StartPIN(). Best regards

Stop autoconnect whenever we're saving a pending DBus message and resume when we return the error response. One event that may be unhandled here is when we cancel the agent request in network_unregister which may happen when the station interface goes DOWN or when the network goes out of range and expires. --- src/network.c | 14 ++++++++++++-- src/station.c | 1 + 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/network.c b/src/network.c index 388bc17a..08f855d2 100644 --- a/src/network.c +++ b/src/network.c @@ -827,6 +827,7 @@ static void passphrase_callback(enum agent_result result, err: network_settings_close(network); + station_set_autoconnect(station, true); } static struct l_dbus_message *network_connect_psk(struct network *network, @@ -862,6 +863,8 @@ static struct l_dbus_message *network_connect_psk(struct network *network, if (!network->agent_request) return dbus_error_no_agent(message); + + station_set_autoconnect(station, false); } else station_connect_network(station, network, bss, message); @@ -992,8 +995,10 @@ static bool eap_send_agent_req(struct network *network, break; } - if (network->agent_request) + if (network->agent_request) { + station_set_autoconnect(network->station, false); return true; + } eap_secret_request_free(req); return false; @@ -1071,6 +1076,7 @@ static void eap_secret_done(enum agent_result result, dbus_pending_reply(&message, dbus_error_no_agent(message)); err: network_settings_close(network); + station_set_autoconnect(network->station, true); } static struct l_dbus_message *network_connect_8021x(struct network *network, @@ -1180,6 +1186,7 @@ static struct l_dbus_message *network_connect(struct l_dbus *dbus, l_dbus_message_ref(message); l_debug("Pending ANQP request, delaying connect to %s", network->ssid); + station_set_autoconnect(station, false); return NULL; } @@ -1530,6 +1537,7 @@ static void anqp_watch_changed(enum station_anqp_state state, reply = dbus_error_not_configured( network->connect_after_anqp); dbus_pending_reply(&network->connect_after_anqp, reply); + station_set_autoconnect(network->station, true); return; } @@ -1537,8 +1545,10 @@ static void anqp_watch_changed(enum station_anqp_state state, network_bss_select(network, true), network->connect_after_anqp); - if (reply) + if (reply) { l_dbus_send(dbus_get_bus(), reply); + station_set_autoconnect(network->station, true); + } l_dbus_message_unref(network->connect_after_anqp); network->connect_after_anqp = NULL; diff --git a/src/station.c b/src/station.c index befc4e25..120a11ea 100644 --- a/src/station.c +++ b/src/station.c @@ -2520,6 +2520,7 @@ void station_connect_network(struct station *station, struct network *network, error: l_dbus_send(dbus, dbus_error_from_errno(err, message)); + station_set_autoconnect(station, true); } static void station_hidden_network_scan_triggered(int err, void *user_data) -- 2.25.1