Module Name:    src
Committed By:   martin
Date:           Tue Apr 17 15:10:53 UTC 2018

Modified Files:
        src/sys/netipsec [netbsd-7]: ipsec_mbuf.c

Log Message:
Pull up following revision(s) (requested by maxv in ticket #1599):

        sys/netipsec/ipsec_mbuf.c: revision 1.23,1.24 (via patch)

Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.

The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().

Fix a pretty bad mistake, that has always been there.

                m_adj(m1, -(m1->m_len - roff));
                if (m1 != m)
                        m->m_pkthdr.len -= (m1->m_len - roff);

This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.

Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.


To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.30.1 src/sys/netipsec/ipsec_mbuf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/netipsec/ipsec_mbuf.c
diff -u src/sys/netipsec/ipsec_mbuf.c:1.12 src/sys/netipsec/ipsec_mbuf.c:1.12.30.1
--- src/sys/netipsec/ipsec_mbuf.c:1.12	Mon May 16 10:05:23 2011
+++ src/sys/netipsec/ipsec_mbuf.c	Tue Apr 17 15:10:53 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: ipsec_mbuf.c,v 1.12 2011/05/16 10:05:23 drochner Exp $	*/
+/*	$NetBSD: ipsec_mbuf.c,v 1.12.30.1 2018/04/17 15:10:53 martin Exp $	*/
 /*-
  * Copyright (c) 2002, 2003 Sam Leffler, Errno Consulting
  * All rights reserved.
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ipsec_mbuf.c,v 1.12 2011/05/16 10:05:23 drochner Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ipsec_mbuf.c,v 1.12.30.1 2018/04/17 15:10:53 martin Exp $");
 
 /*
  * IPsec-specific mbuf routines.
@@ -407,10 +407,11 @@ m_striphdr(struct mbuf *m, int skip, int
 		/* The header was at the beginning of the mbuf */
 		IPSEC_STATINC(IPSEC_STAT_INPUT_FRONT);
 		m_adj(m1, hlen);
-		if ((m1->m_flags & M_PKTHDR) == 0)
+		if (m1 != m)
 			m->m_pkthdr.len -= hlen;
 	} else if (roff + hlen >= m1->m_len) {
 		struct mbuf *mo;
+		int adjlen;
 
 		/*
 		 * Part or all of the header is at the end of this mbuf,
@@ -419,11 +420,13 @@ m_striphdr(struct mbuf *m, int skip, int
 		 */
 		IPSEC_STATINC(IPSEC_STAT_INPUT_END);
 		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...*/
@@ -431,9 +434,10 @@ m_striphdr(struct mbuf *m, int skip, int
 		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) == 0)
-			m->m_pkthdr.len -= (m1->m_len - roff);
+		adjlen = m1->m_len - roff;
+		m_adj(m1, -adjlen);
+		if (m1 != m)
+			m->m_pkthdr.len -= adjlen;
 
 		/* Finally, let's relink */
 		m1->m_next = mo;

Reply via email to