Hi, During fragment reassembly, mbuf chains with packet headers are created. The resulting security bug has been fixed months ago, but the cleanup to avoid the creation of such strange chains is still missing.
See this NetBSD commit: http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/uipc_mbuf.c?only_with_tag=MAIN#rev1.182 This diff is originally from markus@ and already contains feedback from claudio@. ok? bluhm Index: share/man/man9/mbuf.9 =================================================================== RCS file: /data/mirror/openbsd/cvs/src/share/man/man9/mbuf.9,v retrieving revision 1.111 diff -u -p -r1.111 mbuf.9 --- share/man/man9/mbuf.9 11 Feb 2018 00:27:10 -0000 1.111 +++ share/man/man9/mbuf.9 6 Sep 2018 15:44:59 -0000 @@ -35,6 +35,7 @@ .Nm MGET , .Nm m_getclr , .Nm m_gethdr , +.Nm m_removehdr , .Nm m_resethdr , .Nm MGETHDR , .Nm m_prepend , @@ -79,6 +80,8 @@ .Ft struct mbuf * .Fn m_getclr "int how" "int type" .Ft void +.Fn m_removehdr "struct mbuf *m" +.Ft void .Fn m_resethdr "struct mbuf *m" .Ft struct mbuf * .Fn m_gethdr "int how" "int type" @@ -486,11 +489,19 @@ See .Fn m_get for a description of .Fa how . +.It Fn m_removehdr "struct mbuf *m" +Convert a mbuf with packet header to one without. +Delete all +.Xr pf 4 +data and all tags attached to a +.Fa mbuf . +Keep the data and mbuf chain, clear the packet header. .It Fn m_resethdr "struct mbuf *m" -Deletes all +Delete all .Xr pf 4 data and all tags attached to a .Fa mbuf . +Keep the data and mbuf chain, initialize the packet header. .It Fn m_gethdr "int how" "int type" Return a pointer to an mbuf of the type specified after initializing it to contain a packet header. Index: sys/kern/uipc_mbuf.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v retrieving revision 1.256 diff -u -p -r1.256 uipc_mbuf.c --- sys/kern/uipc_mbuf.c 18 Mar 2018 21:25:14 -0000 1.256 +++ sys/kern/uipc_mbuf.c 7 Sep 2018 16:15:15 -0000 @@ -291,15 +291,9 @@ m_inithdr(struct mbuf *m) return (m); } -void -m_resethdr(struct mbuf *m) +static inline void +m_clearhdr(struct mbuf *m) { - int len = m->m_pkthdr.len; - u_int8_t loopcnt = m->m_pkthdr.ph_loopcnt; - - KASSERT(m->m_flags & M_PKTHDR); - m->m_flags &= (M_EXT|M_PKTHDR|M_EOR|M_EXTWR|M_ZEROIZE); - /* delete all mbuf tags to reset the state */ m_tag_delete_chain(m); @@ -307,8 +301,27 @@ m_resethdr(struct mbuf *m) pf_mbuf_unlink_state_key(m); #endif /* NPF > 0 */ - /* like m_inithdr(), but keep any associated data and mbufs */ memset(&m->m_pkthdr, 0, sizeof(m->m_pkthdr)); +} + +void +m_removehdr(struct mbuf *m) +{ + KASSERT(m->m_flags & M_PKTHDR); + m_clearhdr(m); + m->m_flags &= ~M_PKTHDR; +} + +void +m_resethdr(struct mbuf *m) +{ + int len = m->m_pkthdr.len; + u_int8_t loopcnt = m->m_pkthdr.ph_loopcnt; + + KASSERT(m->m_flags & M_PKTHDR); + m->m_flags &= (M_EXT|M_PKTHDR|M_EOR|M_EXTWR|M_ZEROIZE); + m_clearhdr(m); + /* like m_inithdr(), but keep any associated data and mbufs */ m->m_pkthdr.pf.prio = IFQ_DEFPRIO; m->m_pkthdr.len = len; m->m_pkthdr.ph_loopcnt = loopcnt; Index: sys/net/pf_norm.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_norm.c,v retrieving revision 1.212 diff -u -p -r1.212 pf_norm.c --- sys/net/pf_norm.c 4 Sep 2018 20:34:10 -0000 1.212 +++ sys/net/pf_norm.c 7 Sep 2018 15:44:55 -0000 @@ -589,6 +589,7 @@ pf_join_fragment(struct pf_fragment *fra m_adj(m2, frent->fe_len - m2->m_pkthdr.len); pool_put(&pf_frent_pl, frent); pf_nfrents--; + m_removehdr(m2); m_cat(m, m2); } @@ -651,8 +652,10 @@ pf_reassemble(struct mbuf **m0, int dir, m = *m0 = pf_join_fragment(frag); frag = NULL; - if (m->m_flags & M_PKTHDR) { + { int plen = 0; + + KASSERT(m->m_flags & M_PKTHDR); for (m = *m0; m; m = m->m_next) plen += m->m_len; m = *m0; @@ -750,8 +753,10 @@ pf_reassemble6(struct mbuf **m0, struct if (frag6_deletefraghdr(m, hdrlen) != 0) goto fail; - if (m->m_flags & M_PKTHDR) { + { int plen = 0; + + KASSERT(m->m_flags & M_PKTHDR); for (m = *m0; m; m = m->m_next) plen += m->m_len; m = *m0; Index: sys/netinet/ip_input.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v retrieving revision 1.338 diff -u -p -r1.338 ip_input.c --- sys/netinet/ip_input.c 10 Jul 2018 11:34:12 -0000 1.338 +++ sys/netinet/ip_input.c 7 Sep 2018 15:45:56 -0000 @@ -947,6 +947,7 @@ insert: nq = LIST_NEXT(q, ipqe_q); pool_put(&ipqent_pool, q); ip_frags--; + m_removehdr(t); m_cat(m, t); } @@ -963,9 +964,10 @@ insert: pool_put(&ipq_pool, fp); m->m_len += (ip->ip_hl << 2); m->m_data -= (ip->ip_hl << 2); - /* some debugging cruft by sklower, below, will go away soon */ - if (m->m_flags & M_PKTHDR) { /* XXX this should be done elsewhere */ + { int plen = 0; + + KASSERT(m->m_flags & M_PKTHDR); for (t = m; t; t = t->m_next) plen += t->m_len; m->m_pkthdr.len = plen; Index: sys/netinet6/frag6.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/frag6.c,v retrieving revision 1.83 diff -u -p -r1.83 frag6.c --- sys/netinet6/frag6.c 22 Aug 2018 19:48:48 -0000 1.83 +++ sys/netinet6/frag6.c 7 Sep 2018 16:17:27 -0000 @@ -400,6 +400,7 @@ frag6_input(struct mbuf **mp, int *offp, t = t->m_next; t->m_next = af6->ip6af_m; m_adj(t->m_next, af6->ip6af_offset); + m_removehdr(t->m_next); pool_put(&ip6af_pool, af6); } @@ -430,8 +431,10 @@ frag6_input(struct mbuf **mp, int *offp, pool_put(&ip6q_pool, q6); - if (m->m_flags & M_PKTHDR) { /* Isn't it always true? */ + { int plen = 0; + + KASSERT(m->m_flags & M_PKTHDR); for (t = m; t; t = t->m_next) plen += t->m_len; m->m_pkthdr.len = plen; Index: sys/sys/mbuf.h =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v retrieving revision 1.236 diff -u -p -r1.236 mbuf.h --- sys/sys/mbuf.h 10 Jul 2018 09:28:27 -0000 1.236 +++ sys/sys/mbuf.h 7 Sep 2018 15:38:09 -0000 @@ -441,6 +441,7 @@ struct mbuf *m_get(int, int); struct mbuf *m_getclr(int, int); struct mbuf *m_gethdr(int, int); struct mbuf *m_inithdr(struct mbuf *); +void m_removehdr(struct mbuf *); void m_resethdr(struct mbuf *); int m_defrag(struct mbuf *, int); struct mbuf *m_prepend(struct mbuf *, int, int);
