Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: Fw: Re: Connman coredump (Cliff McDiarmid)
2. [PATCH] gsupplicant: Updates to handling of security change
(Harish Jenny K N)
3. Re: [PATCH] gsupplicant: Updates to handling of security
change (Jose Blanquicet)
4. RE: [PATCH] gsupplicant: Updates to handling of security
change (Kandiga, Harish)
5. [PATCH v2] gsupplicant: Updates to handling of security
change (Harish Jenny K N)
----------------------------------------------------------------------
Message: 1
Date: Wed, 27 Sep 2017 21:08:19 +0200
From: "Cliff McDiarmid" <[email protected]>
To: "Daniel Wagner" <[email protected]>, connman <[email protected]>
Subject: Re: Fw: Re: Connman coredump
Message-ID:
<trinity-46f50a3a-a763-4c55-b7a7-de6f627b3d81-1506539299483@3c-app-mailcom-lxa03>
Content-Type: text/plain; charset=UTF-8
Sent:?Wednesday, September 27, 2017 at 8:34 AM
From:?"Daniel Wagner" <[email protected]>
To:?"Cliff McDiarmid" <[email protected]>, [email protected]
Subject:?Re: Fw: Re: Connman coredump
Hi Cliff,
> 'timedatectl status' gives
>
>
> ???? Local time: Tue 2017-09-26 19:30:34 BST
> Universal time: Tue 2017-09-26 18:30:34 UTC
> ????? RTC time: Tue 2017-09-26 18:30:34
> ????? Time zone: Europe/London (BST, +0100)
> Network time on: no
> NTP synchronized: no
> RTC in local TZ: no
>
> Is it possible that this is a bug and ugrading Gilb might help?
>Maybe, but unlikely. But if it is easy to upgrade or try a different
>version it might be worth to try.
> Connman has been a breeze to run in the past.? This IS my first 64bit system.
>What kind of distro are you using? Did you upgrade from a 32bit
>installation to 64bit and keeping the /var/lib/connman directory? If so,
>we just might have to problem, that the history files etc are in 32bit
>and not in 64bit. Just a wild guess.
A wild guess maybe, but dead on. I had moved the /var/lib/connman directory
straight to the new system, as i do a lot of traveling and had many wifi
configs.
I had completely forgotten this move. Many thanks for your patience Daniel,
this is a terrific product.
Cliff
------------------------------
Message: 2
Date: Thu, 28 Sep 2017 12:39:04 +0530
From: Harish Jenny K N <[email protected]>
To: <[email protected]>
Subject: [PATCH] gsupplicant: Updates to handling of security change
Message-ID:
<[email protected]>
Content-Type: text/plain
Changing of security type of one BSS is resulting in removing of
GSupplicantNetwork and other BSSs objects in the group.
The corresponding entries for other BSS in the same group are not
removed in other hash tables like bss_mapping and interface->bss_mapping.
This commit only deletes the network when there is a single entry in
network->bss_table during change in security for one bss.
This commit also avoids accessing any deleted bss object when reply
is got from async dbus call, by deleting any pending dbus calls
associated with the bss object.
---
gsupplicant/supplicant.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 4f79012..7d42404 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -755,6 +755,8 @@ static void remove_bss(gpointer data)
{
struct g_supplicant_bss *bss = data;
+ supplicant_dbus_property_call_cancel_all(bss);
+
g_free(bss->path);
g_free(bss);
}
@@ -2064,7 +2066,7 @@ static void
interface_bss_added_without_keys(DBusMessageIter *iter,
supplicant_dbus_property_get_all(bss->path,
SUPPLICANT_INTERFACE ".BSS",
- bss_property, bss, NULL);
+ bss_property, bss, bss);
bss_compute_security(bss);
if (add_or_replace_bss_to_network(bss) < 0)
@@ -2747,7 +2749,8 @@ static void signal_bss_changed(const char *path,
DBusMessageIter *iter)
if (old_security != bss->security) {
struct g_supplicant_bss *new_bss;
- SUPPLICANT_DBG("New network security for %s", bss->ssid);
+ SUPPLICANT_DBG("New network security for %s with path %s",
+ bss->ssid, bss->path);
/* Security change policy:
* - we first copy the current bss into a new one with
@@ -2767,15 +2770,34 @@ static void signal_bss_changed(const char *path,
DBusMessageIter *iter)
memcpy(new_bss, bss, sizeof(struct g_supplicant_bss));
new_bss->path = g_strdup(bss->path);
- g_hash_table_remove(interface->network_table, network->group);
+ /* Clear the old bss pointer and remove the network completely
+ * if there are no more BSSs in the bss table. The new bss will
+ * be added either to an existing network or an additional
+ * network will be created */
+
+ if (network->best_bss == bss) {
+ network->best_bss = NULL;
+ network->signal = BSS_UNKNOWN_STRENGTH;
+ }
+
+ g_hash_table_remove(bss_mapping, path);
+
+ g_hash_table_remove(interface->bss_mapping, path);
+ g_hash_table_remove(network->bss_table, path);
+
+ update_network_signal(network);
+
+ if (g_hash_table_size(network->bss_table) == 0)
+ g_hash_table_remove(interface->network_table,
+ network->group);
if (add_or_replace_bss_to_network(new_bss) < 0) {
- /* Remove entries in hash tables to handle the
- * failure in add_or_replace_bss_to_network
- */
- g_hash_table_remove(bss_mapping, path);
- g_hash_table_remove(interface->bss_mapping, path);
- g_hash_table_remove(network->bss_table, path);
+ /* Prevent a memory leak on failure in
+ * add_or_replace_bss_to_network */
+ SUPPLICANT_DBG("Failed to add bss %s to network table",
+ new_bss->path);
+ g_free(new_bss->path);
+ g_free(new_bss);
}
return;
--
1.9.1
------------------------------
Message: 3
Date: Thu, 28 Sep 2017 13:49:34 +0200
From: Jose Blanquicet <[email protected]>
To: Harish Jenny K N <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] gsupplicant: Updates to handling of security
change
Message-ID:
<CAFC8iJLbp_13=jzs+okpm4doudgwyjwp5uu8pehkmm6eu-a...@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Hi Harish,
On Thu, Sep 28, 2017 at 9:09 AM, Harish Jenny K N
<[email protected]> wrote:
> Changing of security type of one BSS is resulting in removing of
> GSupplicantNetwork and other BSSs objects in the group.
> The corresponding entries for other BSS in the same group are not
> removed in other hash tables like bss_mapping and interface->bss_mapping.
> This commit only deletes the network when there is a single entry in
> network->bss_table during change in security for one bss.
> This commit also avoids accessing any deleted bss object when reply
> is got from async dbus call, by deleting any pending dbus calls
> associated with the bss object.
>
> ---
> gsupplicant/supplicant.c | 40 +++++++++++++++++++++++++++++++---------
> 1 file changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index 4f79012..7d42404 100644
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -755,6 +755,8 @@ static void remove_bss(gpointer data)
> {
> struct g_supplicant_bss *bss = data;
>
> + supplicant_dbus_property_call_cancel_all(bss);
> +
> g_free(bss->path);
> g_free(bss);
> }
> @@ -2064,7 +2066,7 @@ static void
> interface_bss_added_without_keys(DBusMessageIter *iter,
>
> supplicant_dbus_property_get_all(bss->path,
> SUPPLICANT_INTERFACE ".BSS",
> - bss_property, bss, NULL);
> + bss_property, bss, bss);
>
> bss_compute_security(bss);
> if (add_or_replace_bss_to_network(bss) < 0)
> @@ -2747,7 +2749,8 @@ static void signal_bss_changed(const char *path,
> DBusMessageIter *iter)
> if (old_security != bss->security) {
> struct g_supplicant_bss *new_bss;
>
> - SUPPLICANT_DBG("New network security for %s", bss->ssid);
> + SUPPLICANT_DBG("New network security for %s with path %s",
> + bss->ssid, bss->path);
>
> /* Security change policy:
> * - we first copy the current bss into a new one with
This description is not valid anymore.
> @@ -2767,15 +2770,34 @@ static void signal_bss_changed(const char *path,
> DBusMessageIter *iter)
> memcpy(new_bss, bss, sizeof(struct g_supplicant_bss));
> new_bss->path = g_strdup(bss->path);
>
> - g_hash_table_remove(interface->network_table, network->group);
> + /* Clear the old bss pointer and remove the network completely
> + * if there are no more BSSs in the bss table. The new bss
> will
> + * be added either to an existing network or an additional
> + * network will be created */
I would mix this comment with the one above into a unique one where is
briefly described how things are done now:
/*
* Security change policy:
* - Create a copy ....
* - Remove old BSS's pointers ...
* - Remove network only if there are no more BSSs ...
* ...
*/
> + if (network->best_bss == bss) {
> + network->best_bss = NULL;
> + network->signal = BSS_UNKNOWN_STRENGTH;
> + }
> +
> + g_hash_table_remove(bss_mapping, path);
> +
> + g_hash_table_remove(interface->bss_mapping, path);
> + g_hash_table_remove(network->bss_table, path);
> +
> + update_network_signal(network);
> +
> + if (g_hash_table_size(network->bss_table) == 0)
> + g_hash_table_remove(interface->network_table,
> + network->group);
>
> if (add_or_replace_bss_to_network(new_bss) < 0) {
> - /* Remove entries in hash tables to handle the
> - * failure in add_or_replace_bss_to_network
> - */
> - g_hash_table_remove(bss_mapping, path);
> - g_hash_table_remove(interface->bss_mapping, path);
> - g_hash_table_remove(network->bss_table, path);
> + /* Prevent a memory leak on failure in
> + * add_or_replace_bss_to_network */
> + SUPPLICANT_DBG("Failed to add bss %s to network
> table",
> + new_bss->path);
> + g_free(new_bss->path);
> + g_free(new_bss);
> }
>
> return;
> --
The patch looks fine. I created a simple scenario to test this use
case and the patch does what is intended to.
Thanks,
Jose Blanquicet
------------------------------
Message: 4
Date: Thu, 28 Sep 2017 13:26:27 +0000
From: "Kandiga, Harish" <[email protected]>
To: Jose Blanquicet <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: RE: [PATCH] gsupplicant: Updates to handling of security
change
Message-ID:
<[email protected]>
Content-Type: text/plain; charset="utf-8"
Hi Jose,
>> Changing of security type of one BSS is resulting in removing of
>> GSupplicantNetwork and other BSSs objects in the group.
>> The corresponding entries for other BSS in the same group are not
>> removed in other hash tables like bss_mapping and interface->bss_mapping.
>> This commit only deletes the network when there is a single entry in
>> network->bss_table during change in security for one bss.
>> This commit also avoids accessing any deleted bss object when reply is
>> got from async dbus call, by deleting any pending dbus calls
>> associated with the bss object.
>>
>> ---
>> gsupplicant/supplicant.c | 40
>> +++++++++++++++++++++++++++++++---------
>> 1 file changed, 31 insertions(+), 9 deletions(-)
>>
>> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index
>> 4f79012..7d42404 100644
>> --- a/gsupplicant/supplicant.c
>> +++ b/gsupplicant/supplicant.c
>> @@ -755,6 +755,8 @@ static void remove_bss(gpointer data) {
>> struct g_supplicant_bss *bss = data;
>>
>> + supplicant_dbus_property_call_cancel_all(bss);
>> +
>> g_free(bss->path);
>> g_free(bss);
>> }
>> @@ -2064,7 +2066,7 @@ static void
>> interface_bss_added_without_keys(DBusMessageIter *iter,
>>
>> supplicant_dbus_property_get_all(bss->path,
>> SUPPLICANT_INTERFACE ".BSS",
>> - bss_property, bss, NULL);
>> + bss_property, bss, bss);
>>
>> bss_compute_security(bss);
>> if (add_or_replace_bss_to_network(bss) < 0) @@ -2747,7 +2749,8
>> @@ static void signal_bss_changed(const char *path, DBusMessageIter *iter)
>> if (old_security != bss->security) {
>> struct g_supplicant_bss *new_bss;
>>
>> - SUPPLICANT_DBG("New network security for %s", bss->ssid);
>> + SUPPLICANT_DBG("New network security for %s with path %s",
>> + bss->ssid, bss->path);
>>
>> /* Security change policy:
>> * - we first copy the current bss into a new one with
> This description is not valid anymore.
Agree.
>> @@ -2767,15 +2770,34 @@ static void signal_bss_changed(const char *path,
>> DBusMessageIter *iter)
>> memcpy(new_bss, bss, sizeof(struct g_supplicant_bss));
>> new_bss->path = g_strdup(bss->path);
>>
>> - g_hash_table_remove(interface->network_table,
>> network->group);
>> + /* Clear the old bss pointer and remove the network
>> completely
>> + * if there are no more BSSs in the bss table. The new bss
>> will
>> + * be added either to an existing network or an additional
>> + * network will be created */
> I would mix this comment with the one above into a unique one where is
> briefly described how things are done now:
> /*
> * Security change policy:
> * - Create a copy ....
> * - Remove old BSS's pointers ...
> * - Remove network only if there are no more BSSs ...
> * ...
> */
>> + if (network->best_bss == bss) {
>> + network->best_bss = NULL;
>> + network->signal = BSS_UNKNOWN_STRENGTH;
>> + }
>> +
>> + g_hash_table_remove(bss_mapping, path);
>> +
>> + g_hash_table_remove(interface->bss_mapping, path);
>> + g_hash_table_remove(network->bss_table, path);
>> +
>> + update_network_signal(network);
>> +
>> + if (g_hash_table_size(network->bss_table) == 0)
>> + g_hash_table_remove(interface->network_table,
>> + network->group);
>>
>> if (add_or_replace_bss_to_network(new_bss) < 0) {
>> - /* Remove entries in hash tables to handle the
>> - * failure in add_or_replace_bss_to_network
>> - */
>> - g_hash_table_remove(bss_mapping, path);
>> - g_hash_table_remove(interface->bss_mapping, path);
>> - g_hash_table_remove(network->bss_table, path);
>> + /* Prevent a memory leak on failure in
>> + * add_or_replace_bss_to_network */
>> + SUPPLICANT_DBG("Failed to add bss %s to network
>> table",
>> + new_bss->path);
>> + g_free(new_bss->path);
>> + g_free(new_bss);
>> }
>>
>> return;
>> --
> The patch looks fine. I created a simple scenario to test this use case and
> the patch does what is intended to.
Thanks for confirming. I will resend the second version of patch with updated
comments.
Thanks & Regards,
Harish Jenny K N
------------------------------
Message: 5
Date: Thu, 28 Sep 2017 18:59:58 +0530
From: Harish Jenny K N <[email protected]>
To: <[email protected]>
Subject: [PATCH v2] gsupplicant: Updates to handling of security
change
Message-ID:
<[email protected]>
Content-Type: text/plain
Changing of security type of one BSS is resulting in removing of
GSupplicantNetwork and other BSSs objects in the group.
The corresponding entries for other BSS in the same group are not
removed in other hash tables like bss_mapping and interface->bss_mapping.
This commit only deletes the network when there is a single entry in
network->bss_table during change in security for one bss.
This commit also avoids accessing any deleted bss object when reply
is got from async dbus call, by deleting any pending dbus calls
associated with the bss object.
---
gsupplicant/supplicant.c | 48 +++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 4f79012..5aa6e29 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -755,6 +755,8 @@ static void remove_bss(gpointer data)
{
struct g_supplicant_bss *bss = data;
+ supplicant_dbus_property_call_cancel_all(bss);
+
g_free(bss->path);
g_free(bss);
}
@@ -2064,7 +2066,7 @@ static void
interface_bss_added_without_keys(DBusMessageIter *iter,
supplicant_dbus_property_get_all(bss->path,
SUPPLICANT_INTERFACE ".BSS",
- bss_property, bss, NULL);
+ bss_property, bss, bss);
bss_compute_security(bss);
if (add_or_replace_bss_to_network(bss) < 0)
@@ -2747,18 +2749,16 @@ static void signal_bss_changed(const char *path,
DBusMessageIter *iter)
if (old_security != bss->security) {
struct g_supplicant_bss *new_bss;
- SUPPLICANT_DBG("New network security for %s", bss->ssid);
+ SUPPLICANT_DBG("New network security for %s with path %s",
+ bss->ssid, bss->path);
/* Security change policy:
- * - we first copy the current bss into a new one with
+ * - We first copy the current bss into a new one with
* its own pointer (path)
- * - we remove the current bss related network which will
- * tell the plugin about such removal. This is done due
- * to the fact that a security change means a group change
- * so a complete network change.
- * (current bss becomes invalid as well)
- * - we add the new bss: it adds new network and tell the
- * plugin about it. */
+ * - Clear the old bss pointer and remove the network completely
+ * if there are no more BSSs in the bss table.
+ * - The new bss will be added either to an existing network
+ * or an additional network will be created */
new_bss = g_try_new0(struct g_supplicant_bss, 1);
if (!new_bss)
@@ -2767,15 +2767,29 @@ static void signal_bss_changed(const char *path,
DBusMessageIter *iter)
memcpy(new_bss, bss, sizeof(struct g_supplicant_bss));
new_bss->path = g_strdup(bss->path);
- g_hash_table_remove(interface->network_table, network->group);
+ if (network->best_bss == bss) {
+ network->best_bss = NULL;
+ network->signal = BSS_UNKNOWN_STRENGTH;
+ }
+
+ g_hash_table_remove(bss_mapping, path);
+
+ g_hash_table_remove(interface->bss_mapping, path);
+ g_hash_table_remove(network->bss_table, path);
+
+ update_network_signal(network);
+
+ if (g_hash_table_size(network->bss_table) == 0)
+ g_hash_table_remove(interface->network_table,
+ network->group);
if (add_or_replace_bss_to_network(new_bss) < 0) {
- /* Remove entries in hash tables to handle the
- * failure in add_or_replace_bss_to_network
- */
- g_hash_table_remove(bss_mapping, path);
- g_hash_table_remove(interface->bss_mapping, path);
- g_hash_table_remove(network->bss_table, path);
+ /* Prevent a memory leak on failure in
+ * add_or_replace_bss_to_network */
+ SUPPLICANT_DBG("Failed to add bss %s to network table",
+ new_bss->path);
+ g_free(new_bss->path);
+ g_free(new_bss);
}
return;
--
1.9.1
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 23, Issue 19
***************************************
