changeset 1872:5ffef562088e draft

Ensure routes are initialised correctly, fixes #265. If we fail to malloc any part of a route, clear all the routes.
author Roy Marples <roy@marples.name>
date Sun, 17 Mar 2013 13:00:03 +0000
parents 78c30103009d
children daaec5a8be6b
files dhcp.c ipv4.c
diffstat 2 files changed, 29 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/dhcp.c	Tue Feb 19 16:59:58 2013 +0000
+++ b/dhcp.c	Sun Mar 17 13:00:03 2013 +0000
@@ -497,7 +497,6 @@
 		cidr = *p++;
 		if (cidr > 32) {
 			ipv4_freeroutes(routes);
-			free(routes);
 			errno = EINVAL;
 			return NULL;
 		}
@@ -506,7 +505,6 @@
 		if (rt == NULL) {
 			syslog(LOG_ERR, "%s: %m", __func__);
 			ipv4_freeroutes(routes);
-			free(routes);
 			return NULL;
 		}
 		TAILQ_INSERT_TAIL(routes, rt, next);
@@ -749,6 +747,7 @@
 		syslog(LOG_ERR, "%s: %m", __func__);
 		return NULL;
 	}
+	TAILQ_INIT(routes);
 	if (!has_option_mask(ifo->nomask, DHO_STATICROUTE))
 		p = get_option(dhcp, DHO_STATICROUTE, &len, NULL);
 	else
@@ -759,7 +758,8 @@
 			route = calloc(1, sizeof(*route));
 			if (route == NULL) {
 				syslog(LOG_ERR, "%s: %m", __func__);
-				break;
+				ipv4_freeroutes(routes);
+				return NULL;
 			}
 			memcpy(&route->dest.s_addr, p, 4);
 			p += 4;
@@ -781,7 +781,8 @@
 			route = calloc(1, sizeof(*route));
 			if (route == NULL) {
 				syslog(LOG_ERR, "%s: %m", __func__);
-				break;
+				ipv4_freeroutes(routes);
+				return NULL;
 			}
 			memcpy(&route->gate.s_addr, p, 4);
 			p += 4;
--- a/ipv4.c	Tue Feb 19 16:59:58 2013 +0000
+++ b/ipv4.c	Sun Mar 17 13:00:03 2013 +0000
@@ -341,6 +341,9 @@
 	struct rt *r;
 	const struct dhcp_state *s;
 
+	if (rt == NULL) /* earlier malloc failed */
+		return NULL;
+
 	s = D_CSTATE(ifp);
 	if (s->net.s_addr == INADDR_BROADCAST ||
 	    s->net.s_addr == INADDR_ANY ||
@@ -350,8 +353,11 @@
 		return rt;
 
 	r = malloc(sizeof(*r));
-	if (r == NULL)
+	if (r == NULL) {
+		syslog(LOG_ERR, "%s: %m", __func__);
+		ipv4_freeroutes(rt);
 		return NULL;
+	}
 	r->dest.s_addr = s->addr.s_addr & s->net.s_addr;
 	r->net.s_addr = s->net.s_addr;
 	r->gate.s_addr = 0;
@@ -394,10 +400,12 @@
 {
 	struct rt *r;
 
-	TAILQ_FOREACH(r, rt, next) {
-		if (r->gate.s_addr == D_CSTATE(ifp)->addr.s_addr &&
-		    r->net.s_addr == INADDR_BROADCAST)
-			r->gate.s_addr = r->dest.s_addr;
+	if (rt) {
+		TAILQ_FOREACH(r, rt, next) {
+			if (r->gate.s_addr == D_CSTATE(ifp)->addr.s_addr &&
+			    r->net.s_addr == INADDR_BROADCAST)
+				r->gate.s_addr = r->dest.s_addr;
+		}
 	}
 	return rt;
 }
@@ -407,12 +415,15 @@
 {
 	struct rt *r;
 
-	if (!(iface->flags & IFF_POINTOPOINT) ||
+	if (rt == NULL || /* failed a malloc earlier probably */
+	    !(iface->flags & IFF_POINTOPOINT) ||
 	    !has_option_mask(iface->options->dstmask, DHO_ROUTER))
 		return rt;
+
 	r = malloc(sizeof(*r));
 	if (r == NULL) {
 		syslog(LOG_ERR, "%s: %m", __func__);
+		ipv4_freeroutes(rt);
 		return NULL;
 	}
 	r->dest.s_addr = INADDR_ANY;
@@ -430,6 +441,9 @@
 	struct rt *rtp, *rtn;
 	const char *cp, *cp2, *cp3, *cplim;
 
+	if (rt == NULL) /* earlier malloc failed */
+		return NULL;
+
 	TAILQ_FOREACH(rtp, rt, next) {
 		if (rtp->dest.s_addr != INADDR_ANY)
 			continue;
@@ -466,7 +480,8 @@
 		rtn = malloc(sizeof(*rtn));
 		if (rtn == NULL) {
 			syslog(LOG_ERR, "%s: %m", __func__);
-			continue;
+			ipv4_freeroutes(rt);
+			return NULL;
 		}
 		rtn->dest.s_addr = rtp->gate.s_addr;
 		rtn->net.s_addr = INADDR_BROADCAST;
@@ -501,6 +516,8 @@
 			dnr = add_router_host_route(dnr, ifp);
 			dnr = add_destination_route(dnr, ifp);
 		}
+		if (dnr == NULL) /* failed to malloc all new routes */
+			continue;
 		TAILQ_FOREACH_SAFE(rt, dnr, next, rtn) {
 			rt->iface = ifp;
 			rt->metric = ifp->metric;