11:40 p.m.
I noticed that the Station DBus method GetOrderedNetworks() was not
respecting the behaviour documented in doc/station-api.txt, namely that:
1. If the device is currently connected to a network, that network
is always first on the list,
2. followed by any known networks that have been used at least once
before [...]
These two patches should address the two points respectively.
---
v1 -> v2:
- Fix a careless naming mistake which caused the first patch not to
compile.
Alvin Šipraga (2):
station: refresh ordered network list on (dis)connect
treewide: guard compare functions against signed integer overflow
src/network.c | 2 +-
src/scan.c | 2 +-
src/station.c | 16 ++++++++++++++--
tools/hwsim.c | 2 +-
4 files changed, 17 insertions(+), 5 deletions(-)
--
2.28.0
11:40 p.m.
New subject: [PATCH v2 1/2] station: refresh ordered network list on (dis)connect
Doing so ensures that the currently connected network is always at the
beginning of the list. Previously, the list would only get updated after
a scan.
This fixes the documented behaviour of GetOrderedNetworks() DBus method,
which states that the currently connected network is always at the
beginning of the returned array.
---
src/station.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/station.c b/src/station.c
index 42d60942..a0ec4038 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1195,6 +1195,12 @@ static void station_enter_state(struct station *station,
new_scan_results, station);
break;
case STATION_STATE_CONNECTING:
+ /* Refresh the ordered network list */
+ network_rank_update(station->connected_network, true);
+ l_queue_remove(station->networks_sorted, station->connected_network);
+ l_queue_insert(station->networks_sorted, station->connected_network,
+ network_rank_compare, NULL);
+
l_dbus_property_changed(dbus, netdev_get_path(station->netdev),
IWD_STATION_INTERFACE, "ConnectedNetwork");
l_dbus_property_changed(dbus,
@@ -1295,6 +1301,12 @@ static void station_reset_connection_state(struct station
*station)
station->connected_bss = NULL;
station->connected_network = NULL;
+ /* Refresh the ordered network list */
+ network_rank_update(station->connected_network, false);
+ l_queue_remove(station->networks_sorted, station->connected_network);
+ l_queue_insert(station->networks_sorted, station->connected_network,
+ network_rank_compare, NULL);
+
l_dbus_property_changed(dbus, netdev_get_path(station->netdev),
IWD_STATION_INTERFACE, "ConnectedNetwork");
l_dbus_property_changed(dbus, network_get_path(network),
--
2.28.0
11:40 p.m.
New subject: [PATCH v2 2/2] treewide: guard compare functions against signed integer overflow
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 ****
---
src/network.c | 2 +-
src/scan.c | 2 +-
src/station.c | 4 ++--
tools/hwsim.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/network.c b/src/network.c
index 70541b59..a7d59ca0 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1380,7 +1380,7 @@ int network_rank_compare(const void *a, const void *b, void *user)
const struct network *new_network = a;
const struct network *network = b;
- return network->rank - new_network->rank;
+ return (network->rank > new_network->rank) ? 1 : -1;
}
void network_rank_update(struct network *network, bool connected)
diff --git a/src/scan.c b/src/scan.c
index c93beabd..69e3e975 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -1436,7 +1436,7 @@ int scan_bss_rank_compare(const void *a, const void *b, void
*user_data)
{
const struct scan_bss *new_bss = a, *bss = b;
- return bss->rank - new_bss->rank;
+ return (bss->rank > new_bss->rank) ? 1 : -1;
}
static void get_scan_callback(struct l_genl_msg *msg, void *user_data)
diff --git a/src/station.c b/src/station.c
index a0ec4038..e44dc145 100644
--- a/src/station.c
+++ b/src/station.c
@@ -196,7 +196,7 @@ static int autoconnect_rank_compare(const void *a, const void *b, void
*user)
const struct autoconnect_entry *new_ae = a;
const struct autoconnect_entry *ae = b;
- return ae->rank - new_ae->rank;
+ return (ae->rank > new_ae->rank) ? 1 : -1;
}
static void station_add_autoconnect_bss(struct station *station,
@@ -290,7 +290,7 @@ static int bss_signal_strength_compare(const void *a, const void *b,
void *user)
const struct scan_bss *new_bss = a;
const struct scan_bss *bss = b;
- return bss->signal_strength - new_bss->signal_strength;
+ return (bss->signal_strength > new_bss->signal_strength) ? 1 : -1;
}
/*
diff --git a/tools/hwsim.c b/tools/hwsim.c
index 3ccc936e..02053fa1 100644
--- a/tools/hwsim.c
+++ b/tools/hwsim.c
@@ -1867,7 +1867,7 @@ static int rule_compare_priority(const void *a, const void *b, void
*user)
const struct hwsim_rule *rule_a = a;
const struct hwsim_rule *rule_b = b;
- return rule_a->priority - rule_b->priority;
+ return (rule_a->priority > rule_b->priority) ? 1 : -1;
}
static struct l_dbus_message *rule_add(struct l_dbus *dbus,
--
2.28.0
1:58 a.m.
Hi Alvin,
On 8/14/20 8:40 AM, Alvin Šipraga wrote:
I noticed that the Station DBus method GetOrderedNetworks() was not
respecting the behaviour documented in doc/station-api.txt, namely that:
1. If the device is currently connected to a network, that network
is always first on the list,
2. followed by any known networks that have been used at least once
before [...]
These two patches should address the two points respectively.
---
v1 -> v2:
- Fix a careless naming mistake which caused the first patch not to
compile.
Alvin Šipraga (2):
station: refresh ordered network list on (dis)connect
treewide: guard compare functions against signed integer overflow
src/network.c | 2 +-
src/scan.c | 2 +-
src/station.c | 16 ++++++++++++++--
tools/hwsim.c | 2 +-
4 files changed, 17 insertions(+), 5 deletions(-)
Looks like the bug fixed in patch 2 has been in there since the introduction of
that method. We must have been (un)lucky that the scan is done prior to the
connection and the results look sort of correct.
Good catch and both applied.
Regards,
-Denis
765
days inactive
765
days old
3 comments
2 participants
participants (2)
-
Alvin Šipraga -
Denis Kenzior