Hi Alexandru,

On 07/02/2014 10:18 AM, Alexandru Costache wrote:
> Hi,
> 
> Further testing revealed that d099d36c8654260524c423eff4edcad0171e24aa
> <http://git.kernel.org/cgit/network/connman/connman.git/commit/?id=d099d36c8654260524c423eff4edcad0171e24aa>
> does not fix all crashing conditions when connecting to a network without
> dhcp services. ipv4ll_available_cb() and dhcp_retry_cb() may both fail when
> either dhcp->network is NULL or has just been freed. Removing the timeout
> and the source of the callback when dhcp is invalidated has fixed the rest
> of the failures.

I see.

> 
> I have added this fix as an attachment because I'm not able to use
> git-send-email  for the moment. Please let me know if there's something I
> should change in this patch or if I need to attach it somewhere else.

Okay. Note you can use your gmail account to send this patch as well via
'git send-email'. There are example configuration how to get this
working. I use a msmtp setup to get this working.

> 
> Thanks,
> Alexandru
> 
> 
> 0001-dhcp-Fix-further-crashes.patch
> 
> 
> From 2c0c84abc2fa198f4b88e7b0e14f78e9c26b6f03 Mon Sep 17 00:00:00 2001
> From: Alexandru Costache <[email protected]>
> Date: Tue, 1 Jul 2014 10:42:44 -0400
> Subject: [PATCH] dhcp: Fix further crashes when connected to network without
>  dhcp
> 
> There are several crashing conditions in dhcp_retry_cb() and in
> ipv4ll_available_cb(), when service is NULL or dhcp->network is
> either NULL or has been freed. Fixed all by removing timeout and
> ipv4ll callback when invalidating dhcp.
> ---
>  src/dhcp.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/dhcp.c b/src/dhcp.c
> index c717f84..d20cc3c 100644
> --- a/src/dhcp.c
> +++ b/src/dhcp.c
> @@ -70,6 +70,8 @@ static void dhcp_free(struct connman_dhcp *dhcp)
>       g_free(dhcp);
>  }
>  
> +static void ipv4ll_stop_client(struct connman_dhcp *dhcp);
> +
>  /**
>   * dhcp_invalidate: Invalidate an existing DHCP lease
>   * @dhcp: pointer to the DHCP lease to invalidate.
> @@ -131,6 +133,12 @@ static void dhcp_invalidate(struct connman_dhcp *dhcp, 
> bool callback)
>       __connman_ipconfig_set_gateway(ipconfig, NULL);
>       __connman_ipconfig_set_prefixlen(ipconfig, 0);
>  
> +     if (dhcp->timeout > 0)
> +             g_source_remove(dhcp->timeout);

I think you should also do a 'dhcp->timeout = 0' to be on the safe side.

> +
> +     if (ipv4ll_running)
> +             ipv4ll_stop_client(dhcp);
> +
>       if (dhcp->callback && callback)
>               dhcp->callback(dhcp->network, false, NULL);
>  }
> @@ -221,6 +229,9 @@ static gboolean dhcp_retry_cb(gpointer user_data)
>  
>       dhcp->timeout = 0;
>  
> +     if (!dhcp->network)
> +             return FALSE;
> +

That is not necessary since connman_service_lookup_from_network() has
the exact test from above.

>       service = connman_service_lookup_from_network(dhcp->network);
>       if (!service)
>               return FALSE;
> @@ -471,6 +482,9 @@ static void ipv4ll_available_cb(GDHCPClient 
> *ipv4ll_client, gpointer user_data)
>  
>       DBG("IPV4LL available");
>  
> +     if (!dhcp->network)
> +             return;
> +

Same here.

>       service = connman_service_lookup_from_network(dhcp->network);
>       if (!service)
>               return;


Thanks,
Daniel
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman

Reply via email to