Hi,

On Thu, 2015-07-30 at 16:56 +0200, Peter Meerwald wrote:
> From: Peter Meerwald <p.meerw...@bct-electronic.com>
> 
> we are seeing segfaults when connect_timeout() is called
> 
> (gdb) bt
> 0  0xb6f20f84 in g_str_hash () from /usr/lib/libglib-2.0.so.0
> 1  0xb6f20aa8 in g_hash_table_lookup_extended ()
>    from /usr/lib/libglib-2.0.so.0
> 2  0x0003e87c in allow_property_changed (service=0xda220)
>     at src/service.c:4428
> 3  0x0003effc in dns_changed (service=0xda220) at src/service.c:1950

It seems to me that the service refcount bug is actually the service
unref in __connman_wpad_stop() just before dns_changed() is called in
service.c. This because getting the interface index in
__connman_wpad_stop() means poking into the service struct. Should the
service pointer not exist at this time, the backtrace should contain
__connman_wpad_stop() with the crash in __connman_service_get_index()
instead. So the service pointer must be ok at that point in
__connman_wpad_stop().

That leaves the unreferencing to happen with a successful hash table
removal on the next line in __connman_wpad_stop(). It should remove the
service pointer referenced in the connman_wpad structure, not the one
given as argument. It may happen that the index manages to be set the
same for both services in some circumstances, so we'd better ensure that
the intended service is the one unreferenced.

Then again, the code should not have the same interface index assigned
to both services at the same time, but that is something that may be
unavoidable in certain cases when switching between services. It wasn't
mentioned what else was going on while connect_timeout() was hit, were
there some other services that were connecting or disconnecting as well?

Cheers,

        Patrik

> 4  0x000454ac in service_indicate_state (service=0xda220)
>     at src/service.c:5441
> 5  __connman_service_ipconfig_indicate_state (service=0xda220,
>     new_state=<optimized out>, type=<optimized out>) at src/service.c:5806
> 6  0x00037c58 in set_disconnected (network=0xd3c40) at src/network.c:677
> 7  0x00038784 in __connman_network_disconnect (network=0xd3c40)
>     at src/network.c:1507
> 8  0x000460f8 in connect_timeout (user_data=0xda220) at src/service.c:3955
> 9  0xb6f2c3d4 in ?? () from /usr/lib/libglib-2.0.so.0
> 10 0xb6f2c3d4 in ?? () from /usr/lib/libglib-2.0.so.0
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> the refcount goes to zero while still processing connect_timeout()
> ---
>  src/service.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/service.c b/src/service.c
> index 921a0e4..28aff36 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -3855,6 +3855,7 @@ static void remove_timeout(struct connman_service 
> *service)
>       if (service->timeout > 0) {
>               g_source_remove(service->timeout);
>               service->timeout = 0;
> +             connman_service_unref(service);
>       }
>  }
>  
> @@ -3977,6 +3978,8 @@ static gboolean connect_timeout(gpointer user_data)
>                               CONNMAN_SERVICE_CONNECT_REASON_USER)
>               
> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>  
> +     connman_service_unref(service);
> +
>       return FALSE;
>  }
>  
> @@ -6026,7 +6029,7 @@ int __connman_service_connect(struct connman_service 
> *service,
>       if (err == -EINPROGRESS) {
>               if (service->timeout == 0)
>                       service->timeout = g_timeout_add_seconds(
> -                             CONNECT_TIMEOUT, connect_timeout, service);
> +                             CONNECT_TIMEOUT, connect_timeout, 
> connman_service_ref(service));
>  
>               return -EINPROGRESS;
>       }


_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Reply via email to