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: [PATCH v3 2/2] wifi: p2p: Remove peers when technology is
      disabled. (Patrik Flykt)
   2. Re: [PATCH v3 2/2] wifi: p2p: Remove peers when technology is
      disabled. (John Ernberg)
   3. [PATCH v4 0/2] wifi: p2p: Improvements on p2p and peer
      handling (John Ernberg)
   4. [PATCH v4 1/2] wifi: p2p: Fix several unref of NULL pointer.
      (John Ernberg)
   5. [PATCH v4 2/2] wifi: p2p: Remove peers when technology is
      disabled. (John Ernberg)


----------------------------------------------------------------------

Message: 1
Date: Tue, 22 Dec 2015 10:21:24 +0200
From: Patrik Flykt <[email protected]>
To: John Ernberg <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: [PATCH v3 2/2] wifi: p2p: Remove peers when technology is
        disabled.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"

On Thu, 2015-12-17 at 10:05 +0000, John Ernberg wrote:
> From: John Ernberg <[email protected]>
> 
> If Wi-Fi was disabled, either by removal of a Wi-Fi dongle, or through the
> DBus API, the Wi-Fi technology would sometimes be cleaned up before
> gsupplicant would be signalled by wpa_s that the Peers were removed.
> This means that the discovered peers are not cleaned up properly as there is
> no wifi instance that can handle the callback of the signal.
> Thus the GetPeers() method call could return a list of Peers while Wi-Fi was
> disabled or removed.
> 
> Rewrite the peer-handling to be done more like the networks handling so that
> when the technology is disabled it's possible to loop over the peers and
> free them.

Hrm,

  GEN      src/builtin.h
make --no-print-directory all-am
  CC       plugins/src_connmand-wifi.o
  CC       src/src_connmand-plugin.o
plugins/wifi.c: In function ?peer_connect_timeout?:
plugins/wifi.c:260:37: error: ?struct wifi_data? has no member named ?peer?
     connman_peer_get_identifier(wifi->peer));
                                     ^
plugins/wifi.c: In function ?peer_connect?:
plugins/wifi.c:319:6: error: ?struct wifi_data? has no member named ?peer?
  wifi->peer = NULL;
      ^
Makefile:3027: recipe for target 'plugins/src_connmand-wifi.o' failed
make[1]: *** [plugins/src_connmand-wifi.o] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1602: recipe for target 'all' failed
make: *** [all] Error 2

        Patrik
> ---
>  plugins/wifi.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index 82834ce..da2914f 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -140,7 +140,7 @@ struct wifi_data {
>       unsigned int p2p_find_timeout;
>       unsigned int p2p_connection_timeout;
>       struct connman_peer *pending_peer;
> -     GSupplicantPeer *peer;
> +     GSList *peers;
>       bool p2p_connecting;
>       bool p2p_device;
>       int servicing;
> @@ -245,8 +245,6 @@ static void peer_cancel_timeout(struct wifi_data *wifi)
>               connman_peer_unref(wifi->pending_peer);
>               wifi->pending_peer = NULL;
>       }
> -
> -     wifi->peer = NULL;
>  }
>  
>  static gboolean peer_connect_timeout(gpointer data)
> @@ -257,8 +255,11 @@ static gboolean peer_connect_timeout(gpointer data)
>  
>       if (wifi->p2p_connecting) {
>               enum connman_peer_state state = CONNMAN_PEER_STATE_FAILURE;
> +             GSupplicantPeer *gs_peer =
> +                     g_supplicant_interface_peer_lookup(wifi->interface,
> +                             connman_peer_get_identifier(wifi->peer));
>  
> -             if (g_supplicant_peer_has_requested_connection(wifi->peer))
> +             if (g_supplicant_peer_has_requested_connection(gs_peer))
>                       state = CONNMAN_PEER_STATE_IDLE;
>  
>               connman_peer_set_state(wifi->pending_peer, state);
> @@ -355,7 +356,6 @@ static int peer_connect(struct connman_peer *peer,
>                                               peer_connect_callback, wifi);
>       if (ret == -EINPROGRESS) {
>               wifi->pending_peer = connman_peer_ref(peer);
> -             wifi->peer = gs_peer;
>               wifi->p2p_connecting = true;
>       } else if (ret < 0) {
>               g_free(peer_params->path);
> @@ -794,6 +794,21 @@ static void remove_networks(struct connman_device 
> *device,
>       wifi->networks = NULL;
>  }
>  
> +static void remove_peers(struct wifi_data *wifi)
> +{
> +     GSList *list;
> +
> +     for (list = wifi->peers; list; list = list->next) {
> +             struct connman_peer *peer = list->data;
> +
> +             connman_peer_unregister(peer);
> +             connman_peer_unref(peer);
> +     }
> +
> +     g_slist_free(wifi->peers);
> +     wifi->peers = NULL;
> +}
> +
>  static void reset_autoscan(struct connman_device *device)
>  {
>       struct wifi_data *wifi = connman_device_get_data(device);
> @@ -877,6 +892,7 @@ static void wifi_remove(struct connman_device *device)
>               g_source_remove(wifi->p2p_connection_timeout);
>  
>       remove_networks(device, wifi);
> +     remove_peers(wifi);
>  
>       connman_device_set_powered(device, false);
>       connman_device_set_data(device, NULL);
> @@ -1524,6 +1540,7 @@ static int wifi_disable(struct connman_device *device)
>       }
>  
>       remove_networks(device, wifi);
> +     remove_peers(wifi);
>  
>       ret = g_supplicant_interface_remove(wifi->interface, NULL, NULL);
>       if (ret < 0)
> @@ -2783,6 +2800,8 @@ static void peer_found(GSupplicantPeer *peer)
>       ret = connman_peer_register(connman_peer);
>       if (ret < 0 && ret != -EALREADY)
>               connman_peer_unref(connman_peer);
> +     else
> +             wifi->peers = g_slist_prepend(wifi->peers, connman_peer);
>  }
>  
>  static void peer_lost(GSupplicantPeer *peer)
> @@ -2808,6 +2827,8 @@ static void peer_lost(GSupplicantPeer *peer)
>               connman_peer_unregister(connman_peer);
>               connman_peer_unref(connman_peer);
>       }
> +
> +     wifi->peers = g_slist_remove(wifi->peers, connman_peer);
>  }
>  
>  static void peer_changed(GSupplicantPeer *peer, GSupplicantPeerState state)




------------------------------

Message: 2
Date: Tue, 22 Dec 2015 09:22:33 +0000
From: John Ernberg <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: [PATCH v3 2/2] wifi: p2p: Remove peers when technology is
        disabled.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"



On 12/22/2015 09:21 AM, Patrik Flykt wrote:
> On Thu, 2015-12-17 at 10:05 +0000, John Ernberg wrote:
>> From: John Ernberg <[email protected]>
>>
>> If Wi-Fi was disabled, either by removal of a Wi-Fi dongle, or through the
>> DBus API, the Wi-Fi technology would sometimes be cleaned up before
>> gsupplicant would be signalled by wpa_s that the Peers were removed.
>> This means that the discovered peers are not cleaned up properly as there is
>> no wifi instance that can handle the callback of the signal.
>> Thus the GetPeers() method call could return a list of Peers while Wi-Fi was
>> disabled or removed.
>>
>> Rewrite the peer-handling to be done more like the networks handling so that
>> when the technology is disabled it's possible to loop over the peers and
>> free them.
> Hrm,
>
>    GEN      src/builtin.h
> make --no-print-directory all-am
>    CC       plugins/src_connmand-wifi.o
>    CC       src/src_connmand-plugin.o
> plugins/wifi.c: In function ?peer_connect_timeout?:
> plugins/wifi.c:260:37: error: ?struct wifi_data? has no member named ?peer?
>       connman_peer_get_identifier(wifi->peer));
>                                       ^
> plugins/wifi.c: In function ?peer_connect?:
> plugins/wifi.c:319:6: error: ?struct wifi_data? has no member named ?peer?
>    wifi->peer = NULL;
>        ^
> Makefile:3027: recipe for target 'plugins/src_connmand-wifi.o' failed
> make[1]: *** [plugins/src_connmand-wifi.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1602: recipe for target 'all' failed
> make: *** [all] Error 2
>
>       Patrik
>> ---
>>   plugins/wifi.c | 31 ++++++++++++++++++++++++++-----
>>   1 file changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/plugins/wifi.c b/plugins/wifi.c
>> index 82834ce..da2914f 100644
>> --- a/plugins/wifi.c
>> +++ b/plugins/wifi.c
>> @@ -140,7 +140,7 @@ struct wifi_data {
>>      unsigned int p2p_find_timeout;
>>      unsigned int p2p_connection_timeout;
>>      struct connman_peer *pending_peer;
>> -    GSupplicantPeer *peer;
>> +    GSList *peers;
>>      bool p2p_connecting;
>>      bool p2p_device;
>>      int servicing;
>> @@ -245,8 +245,6 @@ static void peer_cancel_timeout(struct wifi_data *wifi)
>>              connman_peer_unref(wifi->pending_peer);
>>              wifi->pending_peer = NULL;
>>      }
>> -
>> -    wifi->peer = NULL;
>>   }
>>   
>>   static gboolean peer_connect_timeout(gpointer data)
>> @@ -257,8 +255,11 @@ static gboolean peer_connect_timeout(gpointer data)
>>   
>>      if (wifi->p2p_connecting) {
>>              enum connman_peer_state state = CONNMAN_PEER_STATE_FAILURE;
>> +            GSupplicantPeer *gs_peer =
>> +                    g_supplicant_interface_peer_lookup(wifi->interface,
>> +                            connman_peer_get_identifier(wifi->peer));
>>   
>> -            if (g_supplicant_peer_has_requested_connection(wifi->peer))
>> +            if (g_supplicant_peer_has_requested_connection(gs_peer))
>>                      state = CONNMAN_PEER_STATE_IDLE;
>>   
>>              connman_peer_set_state(wifi->pending_peer, state);
>> @@ -355,7 +356,6 @@ static int peer_connect(struct connman_peer *peer,
>>                                              peer_connect_callback, wifi);
>>      if (ret == -EINPROGRESS) {
>>              wifi->pending_peer = connman_peer_ref(peer);
>> -            wifi->peer = gs_peer;
>>              wifi->p2p_connecting = true;
>>      } else if (ret < 0) {
>>              g_free(peer_params->path);
>> @@ -794,6 +794,21 @@ static void remove_networks(struct connman_device 
>> *device,
>>      wifi->networks = NULL;
>>   }
>>   
>> +static void remove_peers(struct wifi_data *wifi)
>> +{
>> +    GSList *list;
>> +
>> +    for (list = wifi->peers; list; list = list->next) {
>> +            struct connman_peer *peer = list->data;
>> +
>> +            connman_peer_unregister(peer);
>> +            connman_peer_unref(peer);
>> +    }
>> +
>> +    g_slist_free(wifi->peers);
>> +    wifi->peers = NULL;
>> +}
>> +
>>   static void reset_autoscan(struct connman_device *device)
>>   {
>>      struct wifi_data *wifi = connman_device_get_data(device);
>> @@ -877,6 +892,7 @@ static void wifi_remove(struct connman_device *device)
>>              g_source_remove(wifi->p2p_connection_timeout);
>>   
>>      remove_networks(device, wifi);
>> +    remove_peers(wifi);
>>   
>>      connman_device_set_powered(device, false);
>>      connman_device_set_data(device, NULL);
>> @@ -1524,6 +1540,7 @@ static int wifi_disable(struct connman_device *device)
>>      }
>>   
>>      remove_networks(device, wifi);
>> +    remove_peers(wifi);
>>   
>>      ret = g_supplicant_interface_remove(wifi->interface, NULL, NULL);
>>      if (ret < 0)
>> @@ -2783,6 +2800,8 @@ static void peer_found(GSupplicantPeer *peer)
>>      ret = connman_peer_register(connman_peer);
>>      if (ret < 0 && ret != -EALREADY)
>>              connman_peer_unref(connman_peer);
>> +    else
>> +            wifi->peers = g_slist_prepend(wifi->peers, connman_peer);
>>   }
>>   
>>   static void peer_lost(GSupplicantPeer *peer)
>> @@ -2808,6 +2827,8 @@ static void peer_lost(GSupplicantPeer *peer)
>>              connman_peer_unregister(connman_peer);
>>              connman_peer_unref(connman_peer);
>>      }
>> +
>> +    wifi->peers = g_slist_remove(wifi->peers, connman_peer);
>>   }
>>   
>>   static void peer_changed(GSupplicantPeer *peer, GSupplicantPeerState state)
>
What horrible things did I do when I exported the patch....?
I build it locally, will check what I messed up.

Best regards // John Ernberg

------------------------------

Message: 3
Date: Tue, 22 Dec 2015 09:30:50 +0000
From: John Ernberg <[email protected]>
To: "[email protected]" <[email protected]>
Subject: [PATCH v4 0/2] wifi: p2p: Improvements on p2p and peer
        handling
Message-ID: <[email protected]>
Content-Type: text/plain; charset="iso-8859-1"

From: John Ernberg <[email protected]>

Horribly messed up v3, patch-set didn't build. Fixed. Sorry.

John Ernberg (2):
  wifi: p2p: Fix several unref of NULL pointer.
  wifi: p2p: Remove peers when technology is disabled.

 plugins/wifi.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 11 deletions(-)

-- 
1.9.1


------------------------------

Message: 4
Date: Tue, 22 Dec 2015 09:30:51 +0000
From: John Ernberg <[email protected]>
To: "[email protected]" <[email protected]>
Subject: [PATCH v4 1/2] wifi: p2p: Fix several unref of NULL pointer.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="iso-8859-1"

From: John Ernberg <[email protected]>

When a Wi-Fi dongle would be removed from the system or if wpa_supplicant
would send a system_killed signal it's possible that the wifi plugin is
destroyed before gsupplicant has reached a state where it will no longer
call any callbacks.
This would result in various SIGSEGV abortions by unref of NULL pointer.

The changes to the p2p scanning were copied from wifi scanning.
---
 plugins/wifi.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 9e9ad64..82834ce 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -309,7 +309,7 @@ static int peer_connect(struct connman_peer *peer,
                return -ENODEV;
 
        wifi = connman_device_get_data(device);
-       if (!wifi)
+       if (!wifi || !wifi->interface)
                return -ENODEV;
 
        if (wifi->p2p_connecting)
@@ -1678,12 +1678,14 @@ static gboolean p2p_find_stop(gpointer data)
 
        DBG("");
 
-       wifi->p2p_find_timeout = 0;
+       if (wifi) {
+               wifi->p2p_find_timeout = 0;
+
+               g_supplicant_interface_p2p_stop_find(wifi->interface);
+       }
 
        connman_device_set_scanning(device, CONNMAN_SERVICE_TYPE_P2P, false);
 
-       g_supplicant_interface_p2p_stop_find(wifi->interface);
-
        connman_device_unref(device);
        reset_autoscan(device);
 
@@ -1698,6 +1700,9 @@ static void p2p_find_callback(int result, 
GSupplicantInterface *interface,
 
        DBG("result %d wifi %p", result, wifi);
 
+       if (!wifi)
+               goto error;
+
        if (wifi->p2p_find_timeout) {
                g_source_remove(wifi->p2p_find_timeout);
                wifi->p2p_find_timeout = 0;
@@ -2512,6 +2517,9 @@ static void p2p_support(GSupplicantInterface *interface)
 
        DBG("");
 
+       if (!interface)
+               return;
+
        if (!g_supplicant_interface_has_p2p(interface))
                return;
 
@@ -2814,6 +2822,9 @@ static void peer_changed(GSupplicantPeer *peer, 
GSupplicantPeerState state)
 
        DBG("ident: %s", identifier);
 
+       if (!wifi)
+               return;
+
        connman_peer = connman_peer_get(wifi->device, identifier);
        if (!connman_peer)
                return;
-- 
1.9.1


------------------------------

Message: 5
Date: Tue, 22 Dec 2015 09:30:51 +0000
From: John Ernberg <[email protected]>
To: "[email protected]" <[email protected]>
Subject: [PATCH v4 2/2] wifi: p2p: Remove peers when technology is
        disabled.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="iso-8859-1"

From: John Ernberg <[email protected]>

If Wi-Fi was disabled, either by removal of a Wi-Fi dongle, or through the
DBus API, the Wi-Fi technology would sometimes be cleaned up before
gsupplicant would be signalled by wpa_s that the Peers were removed.
This means that the discovered peers are not cleaned up properly as there is
no wifi instance that can handle the callback of the signal.
Thus the GetPeers() method call could return a list of Peers while Wi-Fi was
disabled or removed.

Rewrite the peer-handling to be done more like the networks handling so that
when the technology is disabled it's possible to loop over the peers and
free them.
---
 plugins/wifi.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 82834ce..bb1cabc 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -140,7 +140,7 @@ struct wifi_data {
        unsigned int p2p_find_timeout;
        unsigned int p2p_connection_timeout;
        struct connman_peer *pending_peer;
-       GSupplicantPeer *peer;
+       GSList *peers;
        bool p2p_connecting;
        bool p2p_device;
        int servicing;
@@ -245,8 +245,6 @@ static void peer_cancel_timeout(struct wifi_data *wifi)
                connman_peer_unref(wifi->pending_peer);
                wifi->pending_peer = NULL;
        }
-
-       wifi->peer = NULL;
 }
 
 static gboolean peer_connect_timeout(gpointer data)
@@ -257,8 +255,11 @@ static gboolean peer_connect_timeout(gpointer data)
 
        if (wifi->p2p_connecting) {
                enum connman_peer_state state = CONNMAN_PEER_STATE_FAILURE;
+               GSupplicantPeer *gs_peer =
+                       g_supplicant_interface_peer_lookup(wifi->interface,
+                               
connman_peer_get_identifier(wifi->pending_peer));
 
-               if (g_supplicant_peer_has_requested_connection(wifi->peer))
+               if (g_supplicant_peer_has_requested_connection(gs_peer))
                        state = CONNMAN_PEER_STATE_IDLE;
 
                connman_peer_set_state(wifi->pending_peer, state);
@@ -315,8 +316,6 @@ static int peer_connect(struct connman_peer *peer,
        if (wifi->p2p_connecting)
                return -EBUSY;
 
-       wifi->peer = NULL;
-
        gs_peer = g_supplicant_interface_peer_lookup(wifi->interface,
                                        connman_peer_get_identifier(peer));
        if (!gs_peer)
@@ -355,7 +354,6 @@ static int peer_connect(struct connman_peer *peer,
                                                peer_connect_callback, wifi);
        if (ret == -EINPROGRESS) {
                wifi->pending_peer = connman_peer_ref(peer);
-               wifi->peer = gs_peer;
                wifi->p2p_connecting = true;
        } else if (ret < 0) {
                g_free(peer_params->path);
@@ -794,6 +792,21 @@ static void remove_networks(struct connman_device *device,
        wifi->networks = NULL;
 }
 
+static void remove_peers(struct wifi_data *wifi)
+{
+       GSList *list;
+
+       for (list = wifi->peers; list; list = list->next) {
+               struct connman_peer *peer = list->data;
+
+               connman_peer_unregister(peer);
+               connman_peer_unref(peer);
+       }
+
+       g_slist_free(wifi->peers);
+       wifi->peers = NULL;
+}
+
 static void reset_autoscan(struct connman_device *device)
 {
        struct wifi_data *wifi = connman_device_get_data(device);
@@ -877,6 +890,7 @@ static void wifi_remove(struct connman_device *device)
                g_source_remove(wifi->p2p_connection_timeout);
 
        remove_networks(device, wifi);
+       remove_peers(wifi);
 
        connman_device_set_powered(device, false);
        connman_device_set_data(device, NULL);
@@ -1524,6 +1538,7 @@ static int wifi_disable(struct connman_device *device)
        }
 
        remove_networks(device, wifi);
+       remove_peers(wifi);
 
        ret = g_supplicant_interface_remove(wifi->interface, NULL, NULL);
        if (ret < 0)
@@ -2783,6 +2798,8 @@ static void peer_found(GSupplicantPeer *peer)
        ret = connman_peer_register(connman_peer);
        if (ret < 0 && ret != -EALREADY)
                connman_peer_unref(connman_peer);
+       else
+               wifi->peers = g_slist_prepend(wifi->peers, connman_peer);
 }
 
 static void peer_lost(GSupplicantPeer *peer)
@@ -2808,6 +2825,8 @@ static void peer_lost(GSupplicantPeer *peer)
                connman_peer_unregister(connman_peer);
                connman_peer_unref(connman_peer);
        }
+
+       wifi->peers = g_slist_remove(wifi->peers, connman_peer);
 }
 
 static void peer_changed(GSupplicantPeer *peer, GSupplicantPeerState state)
-- 
1.9.1


------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 2, Issue 22
**************************************

Reply via email to