Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/netbsd-8]: src/sys/netipsec Pull up following revision(s) (requested by ...



details:   https://anonhg.NetBSD.org/src/rev/3fd2a1164a61
branches:  netbsd-8
changeset: 434932:3fd2a1164a61
user:      martin <martin%NetBSD.org@localhost>
date:      Tue Apr 17 15:06:20 2018 +0000

description:
Pull up following revision(s) (requested by maxv in ticket #773):

        sys/netipsec/ipsec_mbuf.c: revision 1.23,1.24

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.

diffstat:

 sys/netipsec/ipsec_mbuf.c |  20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diffs (61 lines):

diff -r 17b43e5d0c41 -r 3fd2a1164a61 sys/netipsec/ipsec_mbuf.c
--- a/sys/netipsec/ipsec_mbuf.c Tue Apr 17 08:37:22 2018 +0000
+++ b/sys/netipsec/ipsec_mbuf.c Tue Apr 17 15:06:20 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ipsec_mbuf.c,v 1.16 2017/05/19 04:34:09 ozaki-r Exp $  */
+/*     $NetBSD: ipsec_mbuf.c,v 1.16.2.1 2018/04/17 15:06:20 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.16 2017/05/19 04:34:09 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ipsec_mbuf.c,v 1.16.2.1 2018/04/17 15:06:20 martin Exp $");
 
 /*
  * IPsec-specific mbuf routines.
@@ -403,10 +403,11 @@
                /* 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,
@@ -415,11 +416,13 @@
                 */
                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...*/
@@ -427,9 +430,10 @@
                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;



Home | Main Index | Thread Index | Old Index