Don't alloc/free zero-sized SADB/SPD buffers.

2 files changed, 87 insertions, 73 deletions

diff --git a/usr.sbin/sasyncd/monitor.c b/usr.sbin/sasyncd/monitor.c
index 91baa0bea38..b347be7ffda 100644
--- a/usr.sbin/sasyncd/monitor.c
+++ b/usr.sbin/sasyncd/monitor.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: monitor.c,v 1.1 2005/05/24 02:35:39 ho Exp $	*/
+/*	$OpenBSD: monitor.c,v 1.2 2005/05/26 00:55:03 ho Exp $	*/
 
 /*
  * Copyright (c) 2005 Håkan Olsson.  All rights reserved.
@@ -170,50 +170,62 @@ monitor_get_pfkey_snap(u_int8_t **sadb, u_int32_t *sadbsize, u_int8_t **spd,
 		return -1;
 
 	/* Read SADB data. */
+	*sadb = *spd = NULL;
+	*spdsize = 0;
 	if (read(m_state.s, sadbsize, sizeof *sadbsize) < 1)
 		return -1;
-	*sadb = (u_int8_t *)malloc(*sadbsize);
-	if (!*sadb) {
-		log_err("monitor_get_pfkey_snap: malloc()");
-		/* Drain input */
-		ioctl(m_state.s, FIONBIO, &one);
-		while (read(m_state.s, &tmp, 1) > 0);
-		ioctl(m_state.s, FIONBIO, 0);
-		return -1;
-	}
-	rbytes = read(m_state.s, *sadb, *sadbsize);
-	if (rbytes != *sadbsize) {
-		if (rbytes > 0)
-			memset(*sadb, 0, rbytes);
-		free(*sadb);
-		return -1;
+	if (*sadbsize) {
+		*sadb = (u_int8_t *)malloc(*sadbsize);
+		if (!*sadb) {
+			log_err("monitor_get_pfkey_snap: malloc()");
+			/* Drain input */
+			ioctl(m_state.s, FIONBIO, &one);
+			while (read(m_state.s, &tmp, 1) > 0);
+			ioctl(m_state.s, FIONBIO, 0);
+			return -1;
+		}
+		rbytes = read(m_state.s, *sadb, *sadbsize);
+		if (rbytes != *sadbsize) {
+			if (rbytes > 0)
+				memset(*sadb, 0, rbytes);
+			free(*sadb);
+			return -1;
+		}
 	}
 
 	/* Read SPD data */
 	if (read(m_state.s, spdsize, sizeof *spdsize) < 1) {
-		memset(*sadb, 0, *sadbsize);
-		free(*sadb);
-		return -1;
-	}
-	*spd = (u_int8_t *)malloc(*spdsize);
-	if (!*spd) {
-		log_err("monitor_get_pfkey_snap: malloc()");
-		/* Drain input */
-		ioctl(m_state.s, FIONBIO, &one);
-		while (read(m_state.s, &tmp, 1) > 0);
-		ioctl(m_state.s, FIONBIO, 0);
-		memset(*sadb, 0, *sadbsize);
-		free(*sadb);
+		if (*sadbsize) {
+			memset(*sadb, 0, *sadbsize);
+			free(*sadb);
+		}
 		return -1;
 	}
-	rbytes = read(m_state.s, *spd, *spdsize);
-	if (rbytes != *spdsize) {
-		if (rbytes > 0)
-			memset(*spd, 0, rbytes);
-		memset(*sadb, 0, *sadbsize);
-		free(*spd);
-		free(*sadb);
-		return -1;
+	if (*spdsize) {
+		*spd = (u_int8_t *)malloc(*spdsize);
+		if (!*spd) {
+			log_err("monitor_get_pfkey_snap: malloc()");
+			/* Drain input */
+			ioctl(m_state.s, FIONBIO, &one);
+			while (read(m_state.s, &tmp, 1) > 0);
+			ioctl(m_state.s, FIONBIO, 0);
+			if (*sadbsize) {
+				memset(*sadb, 0, *sadbsize);
+				free(*sadb);
+			}
+			return -1;
+		}
+		rbytes = read(m_state.s, *spd, *spdsize);
+		if (rbytes != *spdsize) {
+			if (rbytes > 0)
+				memset(*spd, 0, rbytes);
+			free(*spd);
+			if (*sadbsize) {
+				memset(*sadb, 0, *sadbsize);
+				free(*sadb);
+			}
+			return -1;
+		}
 	}
 
 	log_msg(5, "monitor_get_pfkey_snap: got %d bytes SADB, %d bytes SPD",
@@ -289,10 +301,9 @@ m_priv_pfkey_snap(int s)
 		log_err("m_priv_pfkey_snap: write");
 		return;
 	}
-	if (sadb_buflen)
+	if (sadb_buflen) {
 		if (write(s, sadb_buf, sadb_buflen) == -1)
 			log_err("m_priv_pfkey_snap: write");
-	if (sadb_buf) {
 		memset(sadb_buf, 0, sadb_buflen);
 		free(sadb_buf);
 	}
@@ -303,10 +314,9 @@ m_priv_pfkey_snap(int s)
 		log_err("m_priv_pfkey_snap: write");
 		return;
 	}
-	if (spd_buflen)
+	if (spd_buflen) {
 		if (write(s, spd_buf, spd_buflen) == -1)
 			log_err("m_priv_pfkey_snap: write");
-	if (spd_buf) {
 		memset(spd_buf, 0, spd_buflen);
 		free(spd_buf);
 	}
diff --git a/usr.sbin/sasyncd/pfkey.c b/usr.sbin/sasyncd/pfkey.c
index fd4ebddc77d..414295a8ae4 100644
--- a/usr.sbin/sasyncd/pfkey.c
+++ b/usr.sbin/sasyncd/pfkey.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfkey.c,v 1.6 2005/05/24 03:15:11 ho Exp $	*/
+/*	$OpenBSD: pfkey.c,v 1.7 2005/05/26 00:55:03 ho Exp $	*/
 
 /*
  * Copyright (c) 2005 Håkan Olsson.  All rights reserved.
@@ -331,44 +331,48 @@ pfkey_snapshot(void *v)
 	}
 
 	/* Parse SADB data */
-	if (sadbsz)
+	if (sadbsz && sadb) {
 		dump_buf(5, sadb, sadbsz, "pfkey_snapshot: SADB data");
-
-	max = sadb + sadbsz;
-	for (next = sadb; next < max; next += m->sadb_msg_len * CHUNK) {
-		m = (struct sadb_msg *)next;
-
-		if (m->sadb_msg_len == 0)
-			break;
-
-		/* Tweak and send this SA to the peer. */
-		m->sadb_msg_type = SADB_ADD;
-
-		/* Allocate a buffer for the msg, net_queue() will free it. */
-		sendbuf = (u_int8_t *)malloc(m->sadb_msg_len * CHUNK);
-		if (sendbuf) {
-			memcpy(sendbuf, m, m->sadb_msg_len * CHUNK);
-			net_queue(p, MSG_PFKEYDATA, sendbuf,
-			    m->sadb_msg_len * CHUNK);
+		max = sadb + sadbsz;
+		for (next = sadb; next < max;
+		     next += m->sadb_msg_len * CHUNK) {
+			m = (struct sadb_msg *)next;
+			if (m->sadb_msg_len == 0)
+				break;
+
+			/* Tweak and send this SA to the peer. */
+			m->sadb_msg_type = SADB_ADD;
+
+			/* XXX Locate lifetime_cur ext and zero bytes */
+
+			/* Allocate msgbuffer, net_queue() will free it. */
+			sendbuf = (u_int8_t *)malloc(m->sadb_msg_len * CHUNK);
+			if (sendbuf) {
+				memcpy(sendbuf, m, m->sadb_msg_len * CHUNK);
+				net_queue(p, MSG_PFKEYDATA, sendbuf,
+				    m->sadb_msg_len * CHUNK);
+			}
 		}
+		memset(sadb, 0, sadbsz);
+		free(sadb);
 	}
-
+	
 	/* Parse SPD data */
-	if (spdsz)
+	if (spdsz && spd) {
 		dump_buf(5, spd, spdsz, "pfkey_snapshot: SPD data");
 
-	max = spd + spdsz;
-	for (next = spd; next < max; next += sizeof(struct ipsec_policy)) {
-		ip = (struct ipsec_policy *)next;
+		max = spd + spdsz;
+		for (next = spd; next < max;
+		     next += sizeof(struct ipsec_policy)) {
+			ip = (struct ipsec_policy *)next;
+			if (ip->ipo_flags & IPSP_POLICY_SOCKET)
+				continue;
+			/* XXX incomplete */
+		}
 
-		if (ip->ipo_flags & IPSP_POLICY_SOCKET)
-			continue;
+		/* Cleanup. */
+		memset(spd, 0, spdsz);
+		free(spd);
 	}
-
-	/* Cleanup. */
-	memset(sadb, 0, sadbsz);
-	free(sadb);
-	memset(spd, 0, spdsz);
-	free(spd);
 	return;
 }