Cleanup IPsec IPComp error handling with consistent goto drop.

from markus@; OK mpi@

1 files changed, 41 insertions, 44 deletions

diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c
index ea1d33d593b..e472bcce9d8 100644
--- a/sys/netinet/ip_ipcomp.c
+++ b/sys/netinet/ip_ipcomp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_ipcomp.c,v 1.60 2018/05/02 21:28:01 bluhm Exp $ */
+/* $OpenBSD: ip_ipcomp.c,v 1.61 2018/05/12 09:38:33 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org)
@@ -208,20 +208,17 @@ ipcomp_input_cb(struct cryptop *crp)
 	m = (struct mbuf *) crp->crp_buf;
 	if (m == NULL) {
 		/* Shouldn't happen... */
-		free(tc, M_XDATA, 0);
-		crypto_freereq(crp);
-		ipcompstat_inc(ipcomps_crypto);
 		DPRINTF(("%s: bogus returned buffer from crypto\n", __func__));
-		return;
+		ipcompstat_inc(ipcomps_crypto);
+		goto droponly;
 	}
 
 	NET_LOCK();
 
 	tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
 	if (tdb == NULL) {
-		free(tc, M_XDATA, 0);
-		ipcompstat_inc(ipcomps_notdb);
 		DPRINTF(("%s: TDB expired while in crypto", __func__));
+		ipcompstat_inc(ipcomps_notdb);
 		goto baddone;
 	}
 
@@ -232,7 +229,6 @@ ipcomp_input_cb(struct cryptop *crp)
 	/* Hard expiration */
 	if ((tdb->tdb_flags & TDBF_BYTES) &&
 	    (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
-		free(tc, M_XDATA, 0);
 		pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
 		tdb_delete(tdb);
 		goto baddone;
@@ -254,13 +250,11 @@ ipcomp_input_cb(struct cryptop *crp)
 			crypto_dispatch(crp);
 			return;
 		}
-		free(tc, M_XDATA, 0);
-		ipcompstat_inc(ipcomps_noxform);
 		DPRINTF(("%s: crypto error %d\n", __func__,
 		    crp->crp_etype));
+		ipcompstat_inc(ipcomps_noxform);
 		goto baddone;
 	}
-	free(tc, M_XDATA, 0);
 
 	/* Length of data after processing */
 	clen = crp->crp_olen;
@@ -269,16 +263,17 @@ ipcomp_input_cb(struct cryptop *crp)
 	m->m_pkthdr.len = clen + hlen + skip;
 
 	if ((m->m_len < skip + hlen) && (m = m_pullup(m, skip + hlen)) == 0) {
+		ipcompstat_inc(ipcomps_hdrops);
 		goto baddone;
 	}
 
 	/* Find the beginning of the IPCOMP header */
 	m1 = m_getptr(m, skip, &roff);
 	if (m1 == NULL) {
-		ipcompstat_inc(ipcomps_hdrops);
 		DPRINTF(("%s: bad mbuf chain, IPCA %s/%08x\n", __func__,
 		    ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
 		    ntohl(tdb->tdb_spi)));
+		ipcompstat_inc(ipcomps_hdrops);
 		goto baddone;
 	}
 	/* Keep the next protocol field */
@@ -337,6 +332,7 @@ ipcomp_input_cb(struct cryptop *crp)
 
 	/* Release the crypto descriptors */
 	crypto_freereq(crp);
+	free(tc, M_XDATA, 0);
 
 	/* Restore the Next Protocol field */
 	m_copyback(m, protoff, sizeof(u_int8_t), &nproto, M_NOWAIT);
@@ -346,12 +342,12 @@ ipcomp_input_cb(struct cryptop *crp)
 	NET_UNLOCK();
 	return;
 
-baddone:
+ baddone:
 	NET_UNLOCK();
-
+ droponly:
 	m_freem(m);
-
 	crypto_freereq(crp);
+	free(tc, M_XDATA, 0);
 }
 
 /*
@@ -362,9 +358,9 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
     int protoff)
 {
 	struct comp_algo *ipcompx = (struct comp_algo *) tdb->tdb_compalgxform;
-	int             hlen;
+	int error, hlen;
 	struct cryptodesc *crdc = NULL;
-	struct cryptop *crp;
+	struct cryptop *crp = NULL;
 	struct tdb_crypto *tc;
 	struct mbuf    *mi;
 #ifdef ENCDEBUG
@@ -405,9 +401,9 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 			DPRINTF(("%s: packet in IPCA %s/%08x got too big\n",
 			    __func__, ipsp_address(&tdb->tdb_dst, buf,
 			    sizeof(buf)), ntohl(tdb->tdb_spi)));
-			m_freem(m);
 			ipcompstat_inc(ipcomps_toobig);
-			return EMSGSIZE;
+			error = EMSGSIZE;
+			goto drop;
 		}
 		break;
 
@@ -418,9 +414,9 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 			DPRINTF(("%s: packet in IPCA %s/%08x got too big\n",
 			    __func__, ipsp_address(&tdb->tdb_dst, buf,
 			    sizeof(buf)), ntohl(tdb->tdb_spi)));
-			m_freem(m);
 			ipcompstat_inc(ipcomps_toobig);
-			return EMSGSIZE;
+			error = EMSGSIZE;
+			goto drop;
 		}
 		break;
 #endif /* INET6 */
@@ -430,9 +426,9 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 		    "IPCA %s/%08x\n", __func__, tdb->tdb_dst.sa.sa_family,
 		    ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
 		    ntohl(tdb->tdb_spi)));
-		m_freem(m);
 		ipcompstat_inc(ipcomps_nopf);
-		return EPFNOSUPPORT;
+		error = EPFNOSUPPORT;
+		goto drop;
 	}
 
 	/* Update the counters */
@@ -445,8 +441,8 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	    (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
 		pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
 		tdb_delete(tdb);
-		m_freem(m);
-		return EINVAL;
+		error = EINVAL;
+		goto drop;
 	}
 	/* Soft byte expiration */
 	if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
@@ -470,8 +466,8 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 			    ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
 			    ntohl(tdb->tdb_spi)));
 			ipcompstat_inc(ipcomps_hdrops);
-			m_freem(m);
-			return ENOBUFS;
+			error = ENOBUFS;
+			goto drop;
 		}
 
 		m_freem(m);
@@ -482,10 +478,10 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	/* Get crypto descriptors */
 	crp = crypto_getreq(1);
 	if (crp == NULL) {
-		m_freem(m);
 		DPRINTF(("%s: failed to acquire crypto descriptors\n", __func__));
 		ipcompstat_inc(ipcomps_crypto);
-		return ENOBUFS;
+		error = ENOBUFS;
+		goto drop;
 	}
 	crdc = &crp->crp_desc[0];
 
@@ -501,11 +497,10 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	/* IPsec-specific opaque crypto info */
 	tc = malloc(sizeof(*tc), M_XDATA, M_NOWAIT | M_ZERO);
 	if (tc == NULL) {
-		m_freem(m);
-		crypto_freereq(crp);
 		DPRINTF(("%s: failed to allocate tdb_crypto\n", __func__));
 		ipcompstat_inc(ipcomps_crypto);
-		return ENOBUFS;
+		error = ENOBUFS;
+		goto drop;
 	}
 
 	tc->tc_spi = tdb->tdb_spi;
@@ -523,6 +518,11 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	crp->crp_sid = tdb->tdb_cryptoid;
 
 	return crypto_dispatch(crp);
+
+ drop:
+	m_freem(m);
+	crypto_freereq(crp);
+	return error;
 }
 
 /*
@@ -552,20 +552,17 @@ ipcomp_output_cb(struct cryptop *crp)
 	m = (struct mbuf *) crp->crp_buf;
 	if (m == NULL) {
 		/* Shouldn't happen... */
-		free(tc, M_XDATA, 0);
-		crypto_freereq(crp);
-		ipcompstat_inc(ipcomps_crypto);
 		DPRINTF(("%s: bogus returned buffer from crypto\n", __func__));
-		return;
+		ipcompstat_inc(ipcomps_crypto);
+		goto droponly;
 	}
 
 	NET_LOCK();
 
 	tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
 	if (tdb == NULL) {
-		free(tc, M_XDATA, 0);
-		ipcompstat_inc(ipcomps_notdb);
 		DPRINTF(("%s: TDB expired while in crypto\n", __func__));
+		ipcompstat_inc(ipcomps_notdb);
 		goto baddone;
 	}
 
@@ -579,16 +576,15 @@ ipcomp_output_cb(struct cryptop *crp)
 			crypto_dispatch(crp);
 			return;
 		}
-		free(tc, M_XDATA, 0);
-		ipcompstat_inc(ipcomps_noxform);
 		DPRINTF(("%s: crypto error %d\n", __func__, crp->crp_etype));
+		ipcompstat_inc(ipcomps_noxform);
 		goto baddone;
 	}
-	free(tc, M_XDATA, 0);
 
 	/* Check sizes. */
 	if (rlen < crp->crp_olen) {
 		/* Compression was useless, we have lost time. */
+		ipcompstat_inc(ipcomps_minlen); /* misnomer, but like to count */
 		crypto_freereq(crp);
 		if (ipsp_process_done(m, tdb))
 			ipcompstat_inc(ipcomps_outfail);
@@ -638,16 +634,17 @@ ipcomp_output_cb(struct cryptop *crp)
 
 	/* Release the crypto descriptor. */
 	crypto_freereq(crp);
+	free(tc, M_XDATA, 0);
 
 	if (ipsp_process_done(m, tdb))
 		ipcompstat_inc(ipcomps_outfail);
 	NET_UNLOCK();
 	return;
 
-baddone:
+ baddone:
 	NET_UNLOCK();
-
+ droponly:
 	m_freem(m);
-
 	crypto_freereq(crp);
+	free(tc, M_XDATA, 0);
 }