Module Name: src Committed By: maxv Date: Fri Mar 9 11:57:38 UTC 2018
Modified Files: src/sys/kern: uipc_mbuf.c src/sys/netinet: ip_reass.c src/sys/netinet6: frag6.c src/sys/sys: mbuf.h Log Message: Remove M_PKTHDR from secondary mbufs when reassembling packets. This is a real problem, because I found at least one component that relies on the fact that only the first mbuf has M_PKTHDR: far from here, in m_splithdr, we don't update m->m_pkthdr.len if M_PKTHDR is found in a secondary mbuf. (The initial intention there was to avoid updating m_pkthdr.len twice, the assumption was that if M_PKTHDR is set then we're dealing with the first mbuf.) Therefore, when handling fragmented IPsec packets (in particular IPv6, IPv4 is a bit more complicated), we may end up with an incorrect m_pkthdr.len after authentication or decryption. In the case of ESP, this can lead to a remote crash on this instruction: m_copydata(m, m->m_pkthdr.len - 3, 3, lastthree); m_pkthdr.len is bigger than the actual mbuf chain. It seems possible to me to trigger this bug even if you don't have the ESP key, because the fragmentation part is outside of the encrypted ESP payload. So if you MITM the target, and intercept an incoming ESP packet (which you can't decrypt), you should be able to forge a new specially-crafted, fragmented packet and stuff the ESP payload (still encrypted, as you intercepted it) into it. The decryption succeeds and the target crashes. To generate a diff of this commit: cvs rdiff -u -r1.181 -r1.182 src/sys/kern/uipc_mbuf.c cvs rdiff -u -r1.13 -r1.14 src/sys/netinet/ip_reass.c cvs rdiff -u -r1.66 -r1.67 src/sys/netinet6/frag6.c cvs rdiff -u -r1.178 -r1.179 src/sys/sys/mbuf.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/kern/uipc_mbuf.c diff -u src/sys/kern/uipc_mbuf.c:1.181 src/sys/kern/uipc_mbuf.c:1.182 --- src/sys/kern/uipc_mbuf.c:1.181 Mon Jan 22 15:05:27 2018 +++ src/sys/kern/uipc_mbuf.c Fri Mar 9 11:57:38 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: uipc_mbuf.c,v 1.181 2018/01/22 15:05:27 maxv Exp $ */ +/* $NetBSD: uipc_mbuf.c,v 1.182 2018/03/09 11:57:38 maxv Exp $ */ /* * Copyright (c) 1999, 2001 The NetBSD Foundation, Inc. @@ -62,7 +62,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.181 2018/01/22 15:05:27 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.182 2018/03/09 11:57:38 maxv Exp $"); #ifdef _KERNEL_OPT #include "opt_mbuftrace.h" @@ -455,6 +455,16 @@ mb_ctor(void *arg, void *object, int fla return (0); } +void +m_pkthdr_remove(struct mbuf *m) +{ + KASSERT(m->m_flags & M_PKTHDR); + + m_tag_delete_chain(m, NULL); + m->m_flags &= ~M_PKTHDR; + memset(&m->m_pkthdr, 0, sizeof(m->m_pkthdr)); +} + /* * Add mbuf to the end of a chain */ Index: src/sys/netinet/ip_reass.c diff -u src/sys/netinet/ip_reass.c:1.13 src/sys/netinet/ip_reass.c:1.14 --- src/sys/netinet/ip_reass.c:1.13 Thu Feb 8 10:03:52 2018 +++ src/sys/netinet/ip_reass.c Fri Mar 9 11:57:38 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: ip_reass.c,v 1.13 2018/02/08 10:03:52 maxv Exp $ */ +/* $NetBSD: ip_reass.c,v 1.14 2018/03/09 11:57:38 maxv Exp $ */ /* * Copyright (c) 1982, 1986, 1988, 1993 @@ -46,7 +46,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ip_reass.c,v 1.13 2018/02/08 10:03:52 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ip_reass.c,v 1.14 2018/03/09 11:57:38 maxv Exp $"); #include <sys/param.h> #include <sys/types.h> @@ -389,6 +389,7 @@ insert: t = q->ipqe_m; nq = TAILQ_NEXT(q, ipqe_q); pool_cache_put(ipfren_cache, q); + m_pkthdr_remove(t); m_cat(m, t); } @@ -406,7 +407,8 @@ insert: m->m_data -= (ip->ip_hl << 2); /* Fix up mbuf. XXX This should be done elsewhere. */ - if (m->m_flags & M_PKTHDR) { + { + KASSERT(m->m_flags & M_PKTHDR); int plen = 0; for (t = m; t; t = t->m_next) { plen += t->m_len; Index: src/sys/netinet6/frag6.c diff -u src/sys/netinet6/frag6.c:1.66 src/sys/netinet6/frag6.c:1.67 --- src/sys/netinet6/frag6.c:1.66 Wed Feb 7 09:53:08 2018 +++ src/sys/netinet6/frag6.c Fri Mar 9 11:57:38 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: frag6.c,v 1.66 2018/02/07 09:53:08 maxv Exp $ */ +/* $NetBSD: frag6.c,v 1.67 2018/03/09 11:57:38 maxv Exp $ */ /* $KAME: frag6.c,v 1.40 2002/05/27 21:40:31 itojun Exp $ */ /* @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: frag6.c,v 1.66 2018/02/07 09:53:08 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: frag6.c,v 1.67 2018/03/09 11:57:38 maxv Exp $"); #ifdef _KERNEL_OPT #include "opt_net_mpsafe.h" @@ -434,6 +434,7 @@ insert: t = t->m_next; t->m_next = af6->ip6af_m; m_adj(t->m_next, af6->ip6af_offset); + m_pkthdr_remove(t->m_next); kmem_intr_free(af6, sizeof(struct ip6asfrag)); af6 = af6dwn; } @@ -472,12 +473,10 @@ insert: kmem_intr_free(q6, sizeof(struct ip6q)); frag6_nfragpackets--; - if (m->m_flags & M_PKTHDR) { /* Isn't it always true? */ + { + KASSERT(m->m_flags & M_PKTHDR); int plen = 0; for (t = m; t; t = t->m_next) { - /* - * XXX XXX Why don't we remove M_PKTHDR? - */ plen += t->m_len; } m->m_pkthdr.len = plen; Index: src/sys/sys/mbuf.h diff -u src/sys/sys/mbuf.h:1.178 src/sys/sys/mbuf.h:1.179 --- src/sys/sys/mbuf.h:1.178 Wed Feb 28 10:30:20 2018 +++ src/sys/sys/mbuf.h Fri Mar 9 11:57:38 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: mbuf.h,v 1.178 2018/02/28 10:30:20 maxv Exp $ */ +/* $NetBSD: mbuf.h,v 1.179 2018/03/09 11:57:38 maxv Exp $ */ /* * Copyright (c) 1996, 1997, 1999, 2001, 2007 The NetBSD Foundation, Inc. @@ -837,6 +837,8 @@ extern struct mowner revoked_mowner; MALLOC_DECLARE(M_MBUF); MALLOC_DECLARE(M_SONAME); +void m_pkthdr_remove(struct mbuf *); + struct mbuf *m_copym(struct mbuf *, int, int, int); struct mbuf *m_copypacket(struct mbuf *, int); struct mbuf *m_devget(char *, int, int, struct ifnet *,