From owner-freebsd-bugs Wed May 30 17: 0:12 2001 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id E047D37B423 for ; Wed, 30 May 2001 17:00:02 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.1/8.11.1) id f4V002m73775; Wed, 30 May 2001 17:00:02 -0700 (PDT) (envelope-from gnats) Received: from whizzo.transsys.com (whizzo.TransSys.COM [144.202.42.10]) by hub.freebsd.org (Postfix) with ESMTP id AAE5A37B422; Wed, 30 May 2001 16:57:09 -0700 (PDT) (envelope-from louie@whizzo.transsys.com) Received: (from louie@localhost) by whizzo.transsys.com (8.11.3/8.11.3) id f4UNv8k14534; Wed, 30 May 2001 19:57:09 -0400 (EDT) (envelope-from louie) Message-Id: <200105302357.f4UNv8k14534@whizzo.transsys.com> Date: Wed, 30 May 2001 19:57:09 -0400 (EDT) From: Louis Mamakos Reply-To: louie@whizzo.transsys.com To: FreeBSD-gnats-submit@freebsd.org Cc: freebsd-net@freebsd.org, darrenr@freebsd.org X-Send-Pr-Version: 3.113 Subject: kern/27782: ipf packet munging bug using "to" option in rule Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org >Number: 27782 >Category: kern >Synopsis: ipf packet munging bug using "to" option in rule >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed May 30 17:00:02 PDT 2001 >Closed-Date: >Last-Modified: >Originator: Louis Mamakos >Release: FreeBSD 4.3-STABLE i386 >Organization: dept of serendipity scheduling and managment >Environment: System: FreeBSD whizzo.transsys.com 4.3-STABLE FreeBSD 4.3-STABLE #18: Sat May 12 20:38:19 EDT 2001 louie@whizzo.transsys.com:/usr/src/sys/compile/SAYSHELL i386 >Description: I'm using ipf on a machine to perform some stupid packet tricks, including the dubious operation of fowarding traffic out an alternative network interface based on the source address of the packet. The reasons why are beyond the scope of this PR. I have a rule that looks something like this: pass out quick on de0 from any to any head 11 block out quick on de0 to tun0 from 144.202.0.0/16 to !144.202.0.0/16 group 11 ... The intent is that when packets from 144.202.0.0/16 arrive on this host, that they get forcebly shoved out the tun0 network interface, rather than taking the expect forwarding path via the de0 interface. This works MOST OF THE TIME. However, sending packets larger than a certain size results in the packet being mangled. It turns out this happens when the packet is large enough that it can no longer be contained within a single mbuf, and there's an mbuf cluster attached. There is code in sys/netinet/ip_fil.c:ipfr_fastroute() which does something like this (paraphrased):n ip->ip_len = htons(ip->ip_len); ip->ip_off = htons(ip->ip_off); if (!ip->ip_sum) ip->ip_sum = in_cksum(m, hlen); error = (*ifp->if_output)(ifp, m, (struct sockaddr *)dst, ro->ro_rt); if (i) { ip->ip_id = ntohs(ip->ip_id); ip->ip_len = ntohs(ip->ip_len); ip->ip_off = ntohs(ip->ip_off); } The problem is that if the outbound network interface saves a pointer to the cluster, it will pick up up the version of the packet after it's been modified by the ntohs() calls after the if_output routine is called. In my case, this happens on a tunnel interface but other network interfaces which can DMA directly from the mbufs rather than copying the data will suffer from the same problem. >How-To-Repeat: as described, sorta.. >Fix: I did a brutal hack in ipfr_fastroute() to make a copy of the mbuf if it's not M_WRITABLE(), and then arranged for the if (i) case in the code above to never get run to corrupt the data in the mbuf. I don't understand the global structure of the ipf code well enough to know if that's a "proper" way to solve the problem or not. After applying the workaround, traffic is now working where previously I was getting nasty checksum errors and the like due to some of the fields being byte-swapped. I would imagine that big-endian systems and those with network interfaces that require the transmit data to be copied wouldn't have seen this problem previously. --- /sys/netinet/ip_fil.c Sun Feb 25 10:53:34 2001 +++ /tmp/ip_fil.c Wed May 30 19:50:45 2001 @@ -1284,6 +1284,32 @@ struct route iproute; frentry_t *fr; +#ifdef __FreeBSD__ + /* + * HOT FIX/KLUDGE: + * + * If the mbuf we're about to send is not writable (because of + * a cluster reference, for example) we'll need to make a copy + * of it since this routine modifies the contents. + * + * If you have non-crappy network hardware that can transmit data + * from the mbuf, rather than making a copy, this is gonna be a + * problem. + */ + + if (M_WRITABLE(m) == 0) { + if ((m0 = m_dup(m, M_DONTWAIT)) != 0) { + m_freem(m); + m = m0; + } else { + error = ENOBUFS; + m_freem(m); + ipl_frouteok[1]++; + return 0; + } + } +#endif + hlen = fin->fin_hlen; ip = mtod(m0, struct ip *); @@ -1379,12 +1405,17 @@ # if BSD >= 199306 int i = 0; +#ifdef __FreeBSD__ + /* HOT FIX: we've already made a copy of the mbuf + above, so we won't need to restore it here. */ +#else /* __FreeBSD__ */ # ifdef MCLISREFERENCED if ((m->m_flags & M_EXT) && MCLISREFERENCED(m)) # else if (m->m_flags & M_EXT) # endif i = 1; +#endif /* __FreeBSD__ */ # endif # ifndef sparc # ifndef __FreeBSD__ @@ -1399,7 +1430,9 @@ error = (*ifp->if_output)(ifp, m, (struct sockaddr *)dst, ro->ro_rt); if (i) { +# ifndef __FreeBSD__ ip->ip_id = ntohs(ip->ip_id); +# endif ip->ip_len = ntohs(ip->ip_len); ip->ip_off = ntohs(ip->ip_off); } >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message