From owner-svn-src-stable-10@freebsd.org Sun Oct 2 21:11:26 2016 Return-Path: Delivered-To: svn-src-stable-10@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id ECE23AC6798; Sun, 2 Oct 2016 21:11:26 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id AC57DA89; Sun, 2 Oct 2016 21:11:26 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u92LBP95058329; Sun, 2 Oct 2016 21:11:25 GMT (envelope-from kp@FreeBSD.org) Received: (from kp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u92LBP1f058328; Sun, 2 Oct 2016 21:11:25 GMT (envelope-from kp@FreeBSD.org) Message-Id: <201610022111.u92LBP1f058328@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kp set sender to kp@FreeBSD.org using -f From: Kristof Provost Date: Sun, 2 Oct 2016 21:11:25 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r306594 - stable/10/sys/net X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-10@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 10-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Oct 2016 21:11:27 -0000 Author: kp Date: Sun Oct 2 21:11:25 2016 New Revision: 306594 URL: https://svnweb.freebsd.org/changeset/base/306594 Log: MFC r306289: bridge: Fix fragment handling and memory leak Fragmented UDP and ICMP packets were corrupted if a firewall with reassembling feature (like pf'scrub) is enabled on the bridge. This patch fixes corrupted packet problem and the panic (triggered easly with low RAM) as explain in PR 185633. bridge_pfil and bridge_fragment relationship: bridge_pfil() receive (IN direction) packets and sent it to the firewall The firewall can be configured for reassembling fragmented packet (like pf'scrubing) in one mbuf chain when bridge_pfil() need to send this reassembled packet to the outgoing interface, it needs to re-fragment it by using bridge_fragment() bridge_fragment() had to split this mbuf (using ip_fragment) first then had to M_PREPEND each packet in the mbuf chain for adding Ethernet header. But M_PREPEND can sometime create a new mbuf on the begining of the mbuf chain, then the "main" pointer of this mbuf chain should be updated and this case is tottaly forgotten. The original bridge_fragment code (Revision 158140, 2006 April 29) came from OpenBSD, and the call to bridge_enqueue was embedded. But on FreeBSD, bridge_enqueue() is done after bridge_fragment(), then the original OpenBSD code can't work as-it of FreeBSD. PR: 185633 Submitted by: Olivier Cochard-Labbé Modified: stable/10/sys/net/if_bridge.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/net/if_bridge.c ============================================================================== --- stable/10/sys/net/if_bridge.c Sun Oct 2 21:06:55 2016 (r306593) +++ stable/10/sys/net/if_bridge.c Sun Oct 2 21:11:25 2016 (r306594) @@ -331,7 +331,7 @@ static int bridge_ip_checkbasic(struct m #ifdef INET6 static int bridge_ip6_checkbasic(struct mbuf **mp); #endif /* INET6 */ -static int bridge_fragment(struct ifnet *, struct mbuf *, +static int bridge_fragment(struct ifnet *, struct mbuf **mp, struct ether_header *, int, struct llc *); static void bridge_linkstate(struct ifnet *ifp); static void bridge_linkcheck(struct bridge_softc *sc); @@ -1862,6 +1862,7 @@ bridge_enqueue(struct bridge_softc *sc, m->m_flags &= ~M_VLANTAG; } + M_ASSERTPKTHDR(m); /* We shouldn't transmit mbuf without pkthdr */ if ((err = dst_ifp->if_transmit(dst_ifp, m))) { m_freem(m0); sc->sc_ifp->if_oerrors++; @@ -3177,10 +3178,12 @@ bridge_pfil(struct mbuf **mp, struct ifn break; /* check if we need to fragment the packet */ + /* bridge_fragment generates a mbuf chain of packets */ + /* that already include eth headers */ if (pfil_member && ifp != NULL && dir == PFIL_OUT) { i = (*mp)->m_pkthdr.len; if (i > ifp->if_mtu) { - error = bridge_fragment(ifp, *mp, &eh2, snap, + error = bridge_fragment(ifp, mp, &eh2, snap, &llc1); return (error); } @@ -3419,56 +3422,77 @@ bad: /* * bridge_fragment: * - * Return a fragmented mbuf chain. + * Fragment mbuf chain in multiple packets and prepend ethernet header. */ static int -bridge_fragment(struct ifnet *ifp, struct mbuf *m, struct ether_header *eh, +bridge_fragment(struct ifnet *ifp, struct mbuf **mp, struct ether_header *eh, int snap, struct llc *llc) { - struct mbuf *m0; + struct mbuf *m = *mp, *nextpkt = NULL, *mprev = NULL, *mcur = NULL; struct ip *ip; int error = -1; if (m->m_len < sizeof(struct ip) && (m = m_pullup(m, sizeof(struct ip))) == NULL) - goto out; + goto dropit; ip = mtod(m, struct ip *); m->m_pkthdr.csum_flags |= CSUM_IP; error = ip_fragment(ip, &m, ifp->if_mtu, ifp->if_hwassist); if (error) - goto out; + goto dropit; - /* walk the chain and re-add the Ethernet header */ - for (m0 = m; m0; m0 = m0->m_nextpkt) { - if (error == 0) { - if (snap) { - M_PREPEND(m0, sizeof(struct llc), M_NOWAIT); - if (m0 == NULL) { - error = ENOBUFS; - continue; - } - bcopy(llc, mtod(m0, caddr_t), - sizeof(struct llc)); - } - M_PREPEND(m0, ETHER_HDR_LEN, M_NOWAIT); - if (m0 == NULL) { + /* + * Walk the chain and re-add the Ethernet header for + * each mbuf packet. + */ + for (mcur = m; mcur; mcur = mcur->m_nextpkt) { + nextpkt = mcur->m_nextpkt; + mcur->m_nextpkt = NULL; + if (snap) { + M_PREPEND(mcur, sizeof(struct llc), M_NOWAIT); + if (mcur == NULL) { error = ENOBUFS; - continue; + if (mprev != NULL) + mprev->m_nextpkt = nextpkt; + goto dropit; } - bcopy(eh, mtod(m0, caddr_t), ETHER_HDR_LEN); - } else - m_freem(m); - } + bcopy(llc, mtod(mcur, caddr_t),sizeof(struct llc)); + } + + M_PREPEND(mcur, ETHER_HDR_LEN, M_NOWAIT); + if (mcur == NULL) { + error = ENOBUFS; + if (mprev != NULL) + mprev->m_nextpkt = nextpkt; + goto dropit; + } + bcopy(eh, mtod(mcur, caddr_t), ETHER_HDR_LEN); - if (error == 0) - KMOD_IPSTAT_INC(ips_fragmented); + /* + * The previous two M_PREPEND could have inserted one or two + * mbufs in front so we have to update the previous packet's + * m_nextpkt. + */ + mcur->m_nextpkt = nextpkt; + if (mprev != NULL) + mprev->m_nextpkt = mcur; + else { + /* The first mbuf in the original chain needs to be + * updated. */ + *mp = mcur; + } + mprev = mcur; + } + KMOD_IPSTAT_INC(ips_fragmented); return (error); -out: - if (m != NULL) - m_freem(m); +dropit: + for (mcur = *mp; mcur; mcur = m) { /* droping the full packet chain */ + m = mcur->m_nextpkt; + m_freem(mcur); + } return (error); }