Do not assume that mbufs within a chain do not have M_PKTHDR set.

This could happen during fragment reassembly.  Better check if we
are dealing with the first mbuf of the chain.
m_adj() changes the length of the mbuf, obviously.  So when using
this length to calulate the amount of adjustment, do not calculate
it again after m_adj() with wrong input.  Use a temporary variable
to save the value.
from Maxime Villard, NetBSD; OK markus@ claudio@

3 files changed, 60 insertions, 23 deletions

diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index fd16674ac93..794e4c09abd 100644
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ah.c,v 1.138 2018/03/14 22:38:46 bluhm Exp $ */
+/*	$OpenBSD: ip_ah.c,v 1.139 2018/05/02 21:28:01 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -855,10 +855,16 @@ ah_input_cb(struct cryptop *crp)
 		 * the mbuf.
 		 */
 		m_adj(m1, rplen + ahx->authsize);
-		if (!(m1->m_flags & M_PKTHDR))
+		/*
+		 * If m1 is the first mbuf, it has set M_PKTHDR and m_adj()
+		 * has already adjusted the packet header length for us.
+		 */
+		if (m1 != m)
 			m->m_pkthdr.len -= rplen + ahx->authsize;
 	} else
 		if (roff + rplen + ahx->authsize >= m1->m_len) {
+			int adjlen;
+
 			/*
 			 * Part or all of the AH header is at the end
 			 * of this mbuf, so first let's remove the
@@ -867,16 +873,16 @@ ah_input_cb(struct cryptop *crp)
 			 * chain, if any.
 			 */
 			if (roff + rplen + ahx->authsize > m1->m_len) {
+				adjlen = roff + rplen + ahx->authsize -
+				    m1->m_len;
 				/* Adjust the next mbuf by the remainder. */
-				m_adj(m1->m_next, roff + rplen +
-				    ahx->authsize - m1->m_len);
+				m_adj(m1->m_next, adjlen);
 
 				/*
 				 * The second mbuf is guaranteed not
 				 * to have a pkthdr...
 				 */
-				m->m_pkthdr.len -=
-				    (roff + rplen + ahx->authsize - m1->m_len);
+				m->m_pkthdr.len -= adjlen;
 			}
 
 			/* Now, let's unlink the mbuf chain for a second... */
@@ -887,9 +893,14 @@ ah_input_cb(struct cryptop *crp)
 			 * ...and trim the end of the first part of
 			 * the chain...sick
 			 */
-			m_adj(m1, -(m1->m_len - roff));
-			if (!(m1->m_flags & M_PKTHDR))
-				m->m_pkthdr.len -= (m1->m_len - roff);
+			adjlen = m1->m_len - roff;
+			m_adj(m1, -adjlen);
+			/*
+			 * If m1 is the first mbuf, it has set M_PKTHDR and
+			 * m_adj() has already adjusted the packet header len.
+			 */
+			if (m1 != m)
+				m->m_pkthdr.len -= adjlen;
 
 			/* Finally, let's relink. */
 			m1->m_next = m0;
diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c
index da62fb80714..4cf21c0ccfa 100644
--- a/sys/netinet/ip_esp.c
+++ b/sys/netinet/ip_esp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_esp.c,v 1.152 2017/11/08 16:29:20 visa Exp $ */
+/*	$OpenBSD: ip_esp.c,v 1.153 2018/05/02 21:28:01 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -673,20 +673,28 @@ esp_input_cb(struct cryptop *crp)
 	if (roff == 0) {
 		/* The ESP header was conveniently at the beginning of the mbuf */
 		m_adj(m1, hlen);
-		if (!(m1->m_flags & M_PKTHDR))
+		/*
+		 * If m1 is the first mbuf, it has set M_PKTHDR and m_adj()
+		 * has already adjusted the packet header length for us.
+		 */
+		if (m1 != m)
 			m->m_pkthdr.len -= hlen;
 	} else if (roff + hlen >= m1->m_len) {
+		int adjlen;
+
 		/*
 		 * Part or all of the ESP header is at the end of this mbuf, so
 		 * first let's remove the remainder of the ESP header from the
 		 * beginning of the remainder of the mbuf chain, if any.
 		 */
 		if (roff + hlen > m1->m_len) {
+			adjlen = roff + hlen - m1->m_len;
+
 			/* Adjust the next mbuf by the remainder */
-			m_adj(m1->m_next, roff + hlen - m1->m_len);
+			m_adj(m1->m_next, adjlen);
 
 			/* The second mbuf is guaranteed not to have a pkthdr */
-			m->m_pkthdr.len -= (roff + hlen - m1->m_len);
+			m->m_pkthdr.len -= adjlen;
 		}
 
 		/* Now, let's unlink the mbuf chain for a second...*/
@@ -694,9 +702,14 @@ esp_input_cb(struct cryptop *crp)
 		m1->m_next = NULL;
 
 		/* ...and trim the end of the first part of the chain...sick */
-		m_adj(m1, -(m1->m_len - roff));
-		if (!(m1->m_flags & M_PKTHDR))
-			m->m_pkthdr.len -= (m1->m_len - roff);
+		adjlen = m1->m_len - roff;
+		m_adj(m1, -adjlen);
+		/*
+		 * If m1 is the first mbuf, it has set M_PKTHDR and m_adj()
+		 * has already adjusted the packet header length for us.
+		 */
+		if (m1 != m)
+			m->m_pkthdr.len -= adjlen;
 
 		/* Finally, let's relink */
 		m1->m_next = mo;
diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c
index 9038e8d5420..ea1d33d593b 100644
--- a/sys/netinet/ip_ipcomp.c
+++ b/sys/netinet/ip_ipcomp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_ipcomp.c,v 1.59 2017/11/08 16:29:20 visa Exp $ */
+/* $OpenBSD: ip_ipcomp.c,v 1.60 2018/05/02 21:28:01 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org)
@@ -290,27 +290,40 @@ ipcomp_input_cb(struct cryptop *crp)
 	if (roff == 0) {
 		/* The IPCOMP header is at the beginning of m1 */
 		m_adj(m1, hlen);
-		if (!(m1->m_flags & M_PKTHDR))
+		/*
+		 * If m1 is the first mbuf, it has set M_PKTHDR and m_adj()
+		 * has already adjusted the packet header length for us.
+		 */
+		if (m1 != m)
 			m->m_pkthdr.len -= hlen;
 	} else if (roff + hlen >= m1->m_len) {
+		int adjlen;
+
 		if (roff + hlen > m1->m_len) {
+			adjlen = roff + hlen - m1->m_len;
+
 			/* Adjust the next mbuf by the remainder */
-			m_adj(m1->m_next, roff + hlen - m1->m_len);
+			m_adj(m1->m_next, adjlen);
 
 			/*
 			 * The second mbuf is guaranteed not to have a
 			 * pkthdr...
 			 */
-			m->m_pkthdr.len -= (roff + hlen - m1->m_len);
+			m->m_pkthdr.len -= adjlen;
 		}
 		/* Now, let's unlink the mbuf chain for a second... */
 		mo = m1->m_next;
 		m1->m_next = NULL;
 
 		/* ...and trim the end of the first part of the chain...sick */
-		m_adj(m1, -(m1->m_len - roff));
-		if (!(m1->m_flags & M_PKTHDR))
-			m->m_pkthdr.len -= (m1->m_len - roff);
+		adjlen = m1->m_len - roff;
+		m_adj(m1, -adjlen);
+		/*
+		 * If m1 is the first mbuf, it has set M_PKTHDR and m_adj()
+		 * has already adjusted the packet header length for us.
+		 */
+		if (m1 != m)
+			m->m_pkthdr.len -= adjlen;
 
 		/* Finally, let's relink */
 		m1->m_next = mo;