Hi Daniel, Thanks for the observations, I've updated the patch accordingly. Sure, I'll set up git mail for the next patches.
Thanks, Alexandru On Wed, Jul 2, 2014 at 11:37 AM, Daniel Wagner <[email protected]> wrote: > 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 >
From 936b41c31d8c3e8413fc53b03354a9c07211d804 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 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/dhcp.c b/src/dhcp.c index c717f84..7048731 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,14 @@ 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); + dhcp->timeout = 0; + } + + if (ipv4ll_running) + ipv4ll_stop_client(dhcp); + if (dhcp->callback && callback) dhcp->callback(dhcp->network, false, NULL); } -- 1.8.1.4
_______________________________________________ connman mailing list [email protected] https://lists.connman.net/mailman/listinfo/connman
