Date: Wed, 30 May 2001 19:57:09 -0400 (EDT) From: Louis Mamakos <louie@whizzo.transsys.com> To: FreeBSD-gnats-submit@freebsd.org Cc: freebsd-net@freebsd.org, darrenr@freebsd.org Subject: ipf packet munging bug using "to" option in rule Message-ID: <200105302357.f4UNv8k14534@whizzo.transsys.com>
next in thread | raw e-mail | index | archive | help
>Submitter-Id: current-users
>Originator: Louis Mamakos
>Organization: dept of serendipity scheduling and managment
>Confidential: no
>Synopsis: ipf packet munging bug using "to" option in rule
>Severity: serious
>Priority: medium
>Category: kern
>Class: sw-bug
>Release: FreeBSD 4.3-STABLE i386
>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);
}
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200105302357.f4UNv8k14534>