dhcpcd-discuss

Hi Brian

On Tuesday 30 August 2016 21:23:03 Gorman, Brian wrote:
> Hi Roy,
> 
> We are running dhcpcd-6.9.1 with a few backported fixes from later versions
> of dhcpcd.
> 
> We have observed a crash on Linux that occurs when the Ethernet link goes
> down and then comes up again as running.
> 
> The crash seems to be a NULL pointer dereference of "state" in dhcp6.c:3274
> (dhcpcd trunk).
> 
> You added a check for a link local address that may have stopped the code
> from getting to this place in this commit, but your comments do not seem
> related:
> 
> http://roy.marples.name/projects/dhcpcd/info/d88b9b636912b1d0
> 
> Do you have any insight into how dhcpcd may have gotten into this point/any
> thoughts on adding a check for a NULL at the point of failure/elsewhere?

Well, dhcp6_start1 is only run directly after state is created or in a LL 
callback (when LL addr DAD completes).

So it's possible that this callback could be firing by mistake.

Does the attached patch help?

Let me know and I'll commit it to trunk.

Roy

--- ipv6.c
+++ ipv6.c
@@ -1623,10 +1623,16 @@
 	if (ifp == NULL)
 		return;
 
 	if ((state = IPV6_STATE(ifp)) == NULL)
 		return;
+
+	/* If we got here, we can get rid of any LL callbacks. */
+	while ((cb = TAILQ_FIRST(&state->ll_callbacks))) {
+		TAILQ_REMOVE(&state->ll_callbacks, cb, next);
+		free(cb);
+	}
 
 	ipv6_freedrop_addrs(&state->addrs, drop ? 2 : 0, NULL);
 	if (drop) {
 		if (ifp->ctx->ipv6 != NULL) {
 			if_initrt6(ifp->ctx);
@@ -1633,14 +1639,10 @@
 			ipv6_buildroutes(ifp->ctx);
 		}
 	} else {
 		/* Because we need to cache the addresses we don't control,
 		 * we only free the state on when NOT dropping addresses. */
-		while ((cb = TAILQ_FIRST(&state->ll_callbacks))) {
-			TAILQ_REMOVE(&state->ll_callbacks, cb, next);
-			free(cb);
-		}
 		free(state);
 		ifp->if_data[IF_DATA_IPV6] = NULL;
 		eloop_timeout_delete(ifp->ctx->eloop, NULL, ifp);
 	}
 }