Attachment 167150 Details for Bug 207087 – Copy the mbuf for use in icmp error messages.

[patch] Copy the mbuf for use in icmp error messages.

tryforwardmcopy.diff (text/plain), 3.76 KB, created by George V. Neville-Neil on 2016-02-18 14:24:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: George V. Neville-Neil

Created: 2016-02-18 14:24:44 UTC

Size: 3.76 KB

patch

obsolete

>Index: sys/netinet/ip_fastfwd.c
>===================================================================
>--- sys/netinet/ip_fastfwd.c	(revision 295703)
>+++ sys/netinet/ip_fastfwd.c	(working copy)
>@@ -165,6 +165,8 @@
> 	int mtu;
> 	struct m_tag *fwd_tag = NULL;
> 
>+	struct mbuf *mcopy;
>+
> 	/*
> 	 * Are we active and forwarding packets?
> 	 */
>@@ -230,6 +232,38 @@
> 	IPSTAT_INC(ips_total);
> 
> 	/*
>+	 * Save the IP header and at most 8 bytes of the payload,
>+	 * in case we need to generate an ICMP message to the src.
>+	 *
>+	 * XXX this can be optimized a lot by saving the data in a local
>+	 * buffer on the stack (72 bytes at most), and only allocating the
>+	 * mbuf if really necessary. The vast majority of the packets
>+	 * are forwarded without having to send an ICMP back (either
>+	 * because unnecessary, or because rate limited), so we are
>+	 * really we are wasting a lot of work here.
>+	 *
>+	 * We don't use m_copy() because it might return a reference
>+	 * to a shared cluster. Both this function and ip_output()
>+	 * assume exclusive access to the IP header in `m', so any
>+	 * data in a cluster may change before we reach icmp_error().
>+	 */
>+	mcopy = m_gethdr(M_NOWAIT, m->m_type);
>+	if (mcopy != NULL && !m_dup_pkthdr(mcopy, m, M_NOWAIT)) {
>+		/*
>+		 * It's probably ok if the pkthdr dup fails (because
>+		 * the deep copy of the tag chain failed), but for now
>+		 * be conservative and just discard the copy since
>+		 * code below may some day want the tags.
>+		 */
>+		m_free(mcopy);
>+		mcopy = NULL;
>+	}
>+	if (mcopy != NULL) {
>+		mcopy->m_len = min(ntohs(ip->ip_len), M_TRAILINGSPACE(mcopy));
>+		mcopy->m_pkthdr.len = mcopy->m_len;
>+		m_copydata(m, 0, mcopy->m_len, mtod(mcopy, caddr_t));
>+	}
>+	/*
> 	 * Step 3: incoming packet firewall processing
> 	 */
> 
>@@ -285,7 +319,9 @@
> 	if (!V_ipstealth) {
> #endif
> 	if (ip->ip_ttl <= IPTTLDEC) {
>-		icmp_error(m, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, 0, 0);
>+		icmp_error(mcopy, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, 0, 0);
>+		if (mcopy)
>+			m_freem(mcopy);
> 		return NULL;	/* mbuf already free'd */
> 	}
> 
>@@ -306,8 +342,11 @@
> 	/*
> 	 * Find route to destination.
> 	 */
>-	if ((dst = ip_findroute(&ro, dest, m)) == NULL)
>+	if ((dst = ip_findroute(&ro, dest, m)) == NULL) {
>+		if (mcopy)
>+			m_freem(mcopy);
> 		return NULL;	/* icmp unreach already sent */
>+	}
> 	ifp = ro.ro_rt->rt_ifp;
> 
> 	/*
>@@ -355,6 +394,8 @@
> 			m->m_flags |= M_FASTFWD_OURS;
> 			if (ro.ro_rt)
> 				RTFREE(ro.ro_rt);
>+			if (mcopy)
>+				m_freem(mcopy);
> 			return m;
> 		}
> 		/*
>@@ -367,8 +408,11 @@
> 			m->m_flags &= ~M_IP_NEXTHOP;
> 		}
> 		RTFREE(ro.ro_rt);
>-		if ((dst = ip_findroute(&ro, dest, m)) == NULL)
>+		if ((dst = ip_findroute(&ro, dest, m)) == NULL) {
>+			if (mcopy)
>+				m_freem(mcopy);
> 			return NULL;	/* icmp unreach already sent */
>+		}
> 		ifp = ro.ro_rt->rt_ifp;
> 	}
> 
>@@ -384,7 +428,7 @@
> 	 */
> 	if ((ro.ro_rt->rt_flags & RTF_REJECT) &&
> 	    (ro.ro_rt->rt_expire == 0 || time_uptime < ro.ro_rt->rt_expire)) {
>-		icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, 0, 0);
>+		icmp_error(mcopy, ICMP_UNREACH, ICMP_UNREACH_HOST, 0, 0);
> 		goto consumed;
> 	}
> 
>@@ -392,7 +436,7 @@
> 	 * Check if media link state of interface is not down
> 	 */
> 	if (ifp->if_link_state == LINK_STATE_DOWN) {
>-		icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, 0, 0);
>+		icmp_error(mcopy, ICMP_UNREACH, ICMP_UNREACH_HOST, 0, 0);
> 		goto consumed;
> 	}
> 
>@@ -421,7 +465,7 @@
> 		 */
> 		if (ip_off & IP_DF) {
> 			IPSTAT_INC(ips_cantfrag);
>-			icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_NEEDFRAG,
>+			icmp_error(mcopy, ICMP_UNREACH, ICMP_UNREACH_NEEDFRAG,
> 				0, mtu);
> 			goto consumed;
> 		} else {
>@@ -469,11 +513,15 @@
> 		IPSTAT_INC(ips_fastforward);
> 	}
> consumed:
>+	if (mcopy)
>+		m_freem(mcopy);
> 	RTFREE(ro.ro_rt);
> 	return NULL;
> drop:
> 	if (m)
> 		m_freem(m);
>+	if (mcopy)
>+		m_freem(mcopy);
> 	if (ro.ro_rt)
> 		RTFREE(ro.ro_rt);
> 	return NULL;

Actions: View | Diff

Attachments on bug 207087: 166844 | 166845 | 166885 | 166886 | 166901 | 166908 | 166909 | 167003 | 167004 | 167113 | 167150