Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/netbsd-7-1]: src/sys/netipsec Pull up following revision(s) (requested b...
details: https://anonhg.NetBSD.org/src/rev/38e3e2399cc1
branches: netbsd-7-1
changeset: 800896:38e3e2399cc1
user: martin <martin%NetBSD.org@localhost>
date: Tue Apr 17 15:38:12 2018 +0000
description:
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.
diffstat:
sys/netipsec/ipsec_mbuf.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diffs (61 lines):
diff -r c8110f9d5bf3 -r 38e3e2399cc1 sys/netipsec/ipsec_mbuf.c
--- a/sys/netipsec/ipsec_mbuf.c Tue Apr 17 08:29:23 2018 +0000
+++ b/sys/netipsec/ipsec_mbuf.c Tue Apr 17 15:38:12 2018 +0000
@@ -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.42.1 2018/04/17 15:38:12 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.42.1 2018/04/17 15:38:12 martin Exp $");
/*
* IPsec-specific mbuf routines.
@@ -407,10 +407,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,
@@ -419,11 +420,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...*/
@@ -431,9 +434,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