From: Roy Marples <roy@marples.name>
Date: Wed, 24 Jun 2015 14:16:05 +0000 (+0000)
Subject: Add a guard to ensure failure to send a DHCP release does not cause a double free... 
X-Git-Tag: v6.9.1~34
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=56db02ab49114524e399c604ced083f561eed892;p=thirdparty%2Fdhcpcd.git

Add a guard to ensure failure to send a DHCP release does not cause a double free via re-entering _drop().
Thanks to Todd Blanchard for the analysis.
Fixes [cd38af54ff]
---

diff --git a/dhcp.c b/dhcp.c
index 56a3859a..e3cfa758 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -2336,6 +2336,12 @@ dhcp_drop(struct interface *ifp, const char *reason)
 	}
 
 	if (ifp->options->options & DHCPCD_RELEASE) {
+		/* Failure to send the release may cause this function to
+		 * re-enter so guard by setting the state. */
+		if (state->state == DHS_RELEASE)
+			return;
+		state->state = DHS_RELEASE;
+
 		unlink(state->leasefile);
 		if (ifp->carrier != LINK_DOWN &&
 		    state->new != NULL &&
diff --git a/dhcp.h b/dhcp.h
index c7dba9d8..b23badf2 100644
--- a/dhcp.h
+++ b/dhcp.h
@@ -194,7 +194,8 @@ enum DHS {
 	DHS_REBIND,
 	DHS_REBOOT,
 	DHS_INFORM,
-	DHS_RENEW_REQUESTED
+	DHS_RENEW_REQUESTED,
+	DHS_RELEASE
 };
 
 struct dhcp_state {
diff --git a/dhcp6.c b/dhcp6.c
index 19f7446b..8fa48abd 100644
--- a/dhcp6.c
+++ b/dhcp6.c
@@ -1219,14 +1219,12 @@ dhcp6_sendconfirm(void *arg)
 	dhcp6_sendmessage(arg, dhcp6_sendconfirm);
 }
 
-/*
 static void
 dhcp6_sendrelease(void *arg)
 {
 
 	dhcp6_sendmessage(arg, dhcp6_sendrelease);
 }
-*/
 
 static void
 dhcp6_startrenew(void *arg)
@@ -1601,6 +1599,18 @@ dhcp6_startexpire(void *arg)
 		    "%s: no advertising IPv6 router wants DHCP", ifp->name);
 }
 
+static void
+dhcp6_finishrelease(void *arg)
+{
+	struct interface *ifp;
+	struct dhcp6_state *state;
+
+	ifp = (struct interface *)arg;
+	state = D6_STATE(ifp);
+	state->state = DH6S_RELEASED;
+	dhcp6_drop(ifp, "RELEASE6");
+}
+
 static void
 dhcp6_startrelease(struct interface *ifp)
 {
@@ -1615,18 +1625,22 @@ dhcp6_startrelease(struct interface *ifp)
 	state->RTC = 0;
 	state->IRT = REL_TIMEOUT;
 	state->MRT = 0;
+	/* MRC of REL_MAX_RC is optional in RFC 3315 18.1.6 */
+#if 0
 	state->MRC = REL_MAX_RC;
-	//state->MRCcallback = dhcp6_failrelease;
+	state->MRCcallback = dhcp6_finishrelease;
+#else
+	state->MRC = 0;
 	state->MRCcallback = NULL;
+#endif
 
 	if (dhcp6_makemessage(ifp) == -1)
 		logger(ifp->ctx, LOG_ERR,
 		    "%s: dhcp6_makemessage: %m", ifp->name);
-	else
-		/* XXX: We should loop a few times
-		 * Luckily RFC3315 section 18.1.6 says this is optional */
-		//dhcp6_sendrelease(ifp);
-		dhcp6_sendmessage(ifp, NULL);
+	else {
+		dhcp6_sendrelease(ifp);
+		dhcp6_finishrelease(ifp);
+	}
 }
 
 static int
@@ -3262,10 +3276,20 @@ dhcp6_freedrop(struct interface *ifp, int drop, const char *reason)
 
 	state = D6_STATE(ifp);
 	if (state) {
-		dhcp_auth_reset(&state->auth);
+		/* Failure to send the release may cause this function to
+		 * re-enter */
+		if (state->state == DH6S_RELEASE) {
+			dhcp6_finishrelease(ifp);
+			return;
+		}
+
 		if (drop && options & DHCPCD_RELEASE) {
-			if (ifp->carrier == LINK_UP)
+			if (ifp->carrier == LINK_UP &&
+			    state->state != DH6S_RELEASED)
+			{
 				dhcp6_startrelease(ifp);
+				return;
+			}
 			unlink(state->leasefile);
 		}
 		dhcp6_freedrop_addrs(ifp, drop, NULL);
diff --git a/dhcp6.h b/dhcp6.h
index 7de5fd75..c32d18ae 100644
--- a/dhcp6.h
+++ b/dhcp6.h
@@ -164,7 +164,8 @@ enum DH6S {
 	DH6S_RENEW_REQUESTED,
 	DH6S_PROBE,
 	DH6S_DELEGATED,
-	DH6S_RELEASE
+	DH6S_RELEASE,
+	DH6S_RELEASED
 };
 
 struct dhcp6_state {