Add a guard to ensure failure to send a DHCP release does not cause a double free...

Thanks to Todd Blanchard for the analysis.
Fixes [cd38af54ff]

diff --git a/dhcp.c b/dhcp.c
index 56a3859abbc511e6f31b1e5964b380c301a0cda9..e3cfa758cf65571ba5d03b8daa3d77fd1e357921 100644 (file)
--- 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 c7dba9d83837ba96ce028a2e993c44fc41c41e21..b23badf266627ef24cc0480290f323d95f3b8023 100644 (file)
--- 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 19f7446ba0158244a9d3b8f001e94d2d7c06bd49..8fa48abdaee439975d90cd1f87b334aab80ce58d 100644 (file)
--- 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 7de5fd75f4b48c7806c179141cc32c943585eaef..c32d18ae322abe5919ef96d34738b84963fb1e9c 100644 (file)
--- 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 {