Date: Tue, 22 May 2012 17:06:04 +0200 From: Daniel Hartmeier <daniel@benzedrine.cx> To: Joerg Pulz <Joerg.Pulz@frm2.tum.de> Cc: freebsd-pf@freebsd.org Subject: Re: kern/168190: [pf] panic when using pf and route-to (maybe: bad fragment handling?) Message-ID: <20120522150603.GF29536@insomnia.benzedrine.cx> In-Reply-To: <201205221200.q4MC0Gtg085514@freefall.freebsd.org> References: <201205221200.q4MC0Gtg085514@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
If you have the chance, please try the patch below.
It adds byte order checks all over the place, hoping for a panic closer
to the source of the problem.
Daniel
Index: sys/sys/mbuf.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/mbuf.h,v
retrieving revision 1.242.2.1
diff -u -r1.242.2.1 mbuf.h
--- sys/sys/mbuf.h 23 Sep 2011 00:51:37 -0000 1.242.2.1
+++ sys/sys/mbuf.h 22 May 2012 14:15:00 -0000
@@ -824,6 +824,20 @@
/* Compatibility with 4.3. */
#define m_copy(m, o, l) m_copym((m), (o), (l), M_DONTWAIT)
+#define ASSERT_NET_BYTE_ORDER(m) do { \
+ struct ip *ip = mtod((m), struct ip *); \
+ if (ip->ip_len != htons(ip->ip_len) && \
+ ip->ip_len == (m)->m_pkthdr.len) \
+ panic("ASSERT_NET_BYTE_ORDER"); \
+} while(0)
+
+#define ASSERT_HOST_BYTE_ORDER(m) do { \
+ struct ip *ip = mtod((m), struct ip *); \
+ if (ip->ip_len != htons(ip->ip_len) && \
+ ntohs(ip->ip_len) == (m)->m_pkthdr.len) \
+ panic("ASSERT_NET_BYTE_ORDER"); \
+} while(0)
+
extern int max_datalen; /* MHLEN - max_hdr */
extern int max_hdr; /* Largest link + protocol header */
extern int max_linkhdr; /* Largest link-level header */
Index: sys/contrib/pf/net/pf.c
===================================================================
RCS file: /home/ncvs/src/sys/contrib/pf/net/pf.c,v
retrieving revision 1.78.2.6
diff -u -r1.78.2.6 pf.c
--- sys/contrib/pf/net/pf.c 29 Feb 2012 09:47:26 -0000 1.78.2.6
+++ sys/contrib/pf/net/pf.c 22 May 2012 14:39:04 -0000
@@ -2560,6 +2560,7 @@
case AF_INET:
#ifdef __FreeBSD__
/* icmp_error() expects host byte ordering */
+ ASSERT_NET_BYTE_ORDER(m0);
ip = mtod(m0, struct ip *);
NTOHS(ip->ip_len);
NTOHS(ip->ip_off);
@@ -5894,6 +5895,8 @@
(dir != PF_IN && dir != PF_OUT) || oifp == NULL)
panic("pf_route: invalid parameters");
+ ASSERT_NET_BYTE_ORDER(*m);
+
#ifdef __FreeBSD__
if (pd->pf_mtag->routed++ > 3) {
#else
@@ -5977,6 +5980,7 @@
if (oifp != ifp) {
#ifdef __FreeBSD__
+ ASSERT_NET_BYTE_ORDER(m0);
PF_UNLOCK();
if (pf_test(PF_OUT, ifp, &m0, NULL, NULL) != PF_PASS) {
PF_LOCK();
@@ -5998,6 +6002,7 @@
goto bad;
}
ip = mtod(m0, struct ip *);
+ ASSERT_NET_BYTE_ORDER(m0);
}
#ifdef __FreeBSD__
@@ -6008,6 +6013,7 @@
/*
* XXX: in_delayed_cksum assumes HBO for ip->ip_len (at least)
*/
+ ASSERT_NET_BYTE_ORDER(m0);
NTOHS(ip->ip_len);
NTOHS(ip->ip_off); /* XXX: needed? */
in_delayed_cksum(m0);
@@ -6017,6 +6023,8 @@
}
m0->m_pkthdr.csum_flags &= ifp->if_hwassist;
+ ASSERT_NET_BYTE_ORDER(m0);
+
if (ntohs(ip->ip_len) <= ifp->if_mtu ||
(ifp->if_hwassist & CSUM_FRAGMENT &&
((ip->ip_off & htons(IP_DF)) == 0))) {
@@ -6104,6 +6112,7 @@
if (r->rt != PF_DUPTO) {
#ifdef __FreeBSD__
/* icmp_error() expects host byte ordering */
+ ASSERT_NET_BYTE_ORDER(m0);
NTOHS(ip->ip_len);
NTOHS(ip->ip_off);
PF_UNLOCK();
@@ -6124,6 +6133,7 @@
/*
* XXX: is cheaper + less error prone than own function
*/
+ ASSERT_NET_BYTE_ORDER(m0);
NTOHS(ip->ip_len);
NTOHS(ip->ip_off);
error = ip_fragment(ip, &m0, ifp->if_mtu, ifp->if_hwassist, sw_csum);
@@ -6672,6 +6682,8 @@
#endif /* DIAGNOSTIC */
#endif
+ ASSERT_NET_BYTE_ORDER(m);
+
if (m->m_pkthdr.len < (int)sizeof(*h)) {
action = PF_DROP;
REASON_SET(&reason, PFRES_SHORT);
Index: sys/contrib/pf/net/pf_ioctl.c
===================================================================
RCS file: /home/ncvs/src/sys/contrib/pf/net/pf_ioctl.c,v
retrieving revision 1.50.2.4
diff -u -r1.50.2.4 pf_ioctl.c
--- sys/contrib/pf/net/pf_ioctl.c 29 Feb 2012 09:47:26 -0000 1.50.2.4
+++ sys/contrib/pf/net/pf_ioctl.c 22 May 2012 14:37:44 -0000
@@ -4121,6 +4121,7 @@
if ((*m)->m_pkthdr.len >= (int)sizeof(struct ip)) {
/* if m_pkthdr.len is less than ip header, pf will handle. */
+ ASSERT_HOST_BYTE_ORDER(*m);
h = mtod(*m, struct ip *);
HTONS(h->ip_len);
HTONS(h->ip_off);
@@ -4134,6 +4135,7 @@
}
if (*m != NULL) {
/* pf_test can change ip header location */
+ ASSERT_NET_BYTE_ORDER(*m);
h = mtod(*m, struct ip *);
NTOHS(h->ip_len);
NTOHS(h->ip_off);
@@ -4163,6 +4165,7 @@
}
if ((*m)->m_pkthdr.len >= (int)sizeof(*h)) {
/* if m_pkthdr.len is less than ip header, pf will handle. */
+ ASSERT_HOST_BYTE_ORDER(*m);
h = mtod(*m, struct ip *);
HTONS(h->ip_len);
HTONS(h->ip_off);
@@ -4176,6 +4179,7 @@
}
if (*m != NULL) {
/* pf_test can change ip header location */
+ ASSERT_NET_BYTE_ORDER(*m);
h = mtod(*m, struct ip *);
NTOHS(h->ip_len);
NTOHS(h->ip_off);
Index: sys/contrib/pf/net/pf_norm.c
===================================================================
RCS file: /home/ncvs/src/sys/contrib/pf/net/pf_norm.c,v
retrieving revision 1.21.2.2
diff -u -r1.21.2.2 pf_norm.c
--- sys/contrib/pf/net/pf_norm.c 29 Feb 2012 09:47:26 -0000 1.21.2.2
+++ sys/contrib/pf/net/pf_norm.c 22 May 2012 14:41:02 -0000
@@ -1190,6 +1190,8 @@
if (hlen < (int)sizeof(struct ip))
goto drop;
+ ASSERT_NET_BYTE_ORDER(m);
+
if (hlen > ntohs(h->ip_len))
goto drop;
Index: sys/net/if_bridge.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_bridge.c,v
retrieving revision 1.144.2.2
diff -u -r1.144.2.2 if_bridge.c
--- sys/net/if_bridge.c 17 Mar 2012 12:11:53 -0000 1.144.2.2
+++ sys/net/if_bridge.c 22 May 2012 14:44:14 -0000
@@ -3142,6 +3142,7 @@
*/
ip = mtod(*mp, struct ip *);
+ ASSERT_NET_BYTE_ORDER(*mp);
ip->ip_len = ntohs(ip->ip_len);
ip->ip_off = ntohs(ip->ip_off);
@@ -3195,6 +3196,7 @@
if (ip == NULL)
goto bad;
}
+ ASSERT_HOST_BYTE_ORDER(*mp);
ip->ip_len = htons(ip->ip_len);
ip->ip_off = htons(ip->ip_off);
ip->ip_sum = 0;
@@ -3332,6 +3334,7 @@
}
/* Retrieve the packet length. */
+ ASSERT_NET_BYTE_ORDER(m);
len = ntohs(ip->ip_len);
/*
Index: sys/net/if_enc.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_enc.c,v
retrieving revision 1.17.2.1
diff -u -r1.17.2.1 if_enc.c
--- sys/net/if_enc.c 23 Sep 2011 00:51:37 -0000 1.17.2.1
+++ sys/net/if_enc.c 22 May 2012 14:43:27 -0000
@@ -274,6 +274,7 @@
* before calling the firewall, swap fields the same as
* IP does. here we assume the header is contiguous
*/
+ ASSERT_NET_BYTE_ORDER(*mp);
ip->ip_len = ntohs(ip->ip_len);
ip->ip_off = ntohs(ip->ip_off);
@@ -284,6 +285,7 @@
break;
/* restore byte ordering */
+ ASSERT_HOST_BYTE_ORDER(*mp);
ip = mtod(*mp, struct ip *);
ip->ip_len = htons(ip->ip_len);
ip->ip_off = htons(ip->ip_off);
Index: sys/net/pfil.c
===================================================================
RCS file: /home/ncvs/src/sys/net/pfil.c,v
retrieving revision 1.19.2.1
diff -u -r1.19.2.1 pfil.c
--- sys/net/pfil.c 23 Sep 2011 00:51:37 -0000 1.19.2.1
+++ sys/net/pfil.c 22 May 2012 14:49:24 -0000
@@ -46,6 +46,8 @@
#include <net/if.h>
#include <net/pfil.h>
+#include <netinet/in.h>
+#include <netinet/ip.h>
static struct mtx pfil_global_lock;
@@ -79,10 +81,12 @@
for (pfh = pfil_hook_get(dir, ph); pfh != NULL;
pfh = TAILQ_NEXT(pfh, pfil_link)) {
if (pfh->pfil_func != NULL) {
+ ASSERT_HOST_BYTE_ORDER(m);
rv = (*pfh->pfil_func)(pfh->pfil_arg, &m, ifp, dir,
inp);
if (rv != 0 || m == NULL)
break;
+ ASSERT_HOST_BYTE_ORDER(m);
}
}
PFIL_RUNLOCK(ph, &rmpt);
Index: sys/netinet/ip_divert.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.173.2.1
diff -u -r1.173.2.1 ip_divert.c
--- sys/netinet/ip_divert.c 23 Sep 2011 00:51:37 -0000 1.173.2.1
+++ sys/netinet/ip_divert.c 22 May 2012 14:27:15 -0000
@@ -207,6 +207,7 @@
(m = m_pullup(m, sizeof(struct ip))) == 0)
return;
ip = mtod(m, struct ip *);
+ ASSERT_NET_BYTE_ORDER(m);
/* Delayed checksums are currently not compatible with divert. */
if (m->m_pkthdr.csum_flags & CSUM_DELAY_DATA) {
@@ -396,6 +397,7 @@
/* Convert fields to host order for ip_output() */
ip->ip_len = ntohs(ip->ip_len);
ip->ip_off = ntohs(ip->ip_off);
+ ASSERT_HOST_BYTE_ORDER(m);
break;
#ifdef INET6
case IPV6_VERSION >> 4:
Index: sys/netinet/ip_fastfwd.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_fastfwd.c,v
retrieving revision 1.57.2.1
diff -u -r1.57.2.1 ip_fastfwd.c
--- sys/netinet/ip_fastfwd.c 23 Sep 2011 00:51:37 -0000 1.57.2.1
+++ sys/netinet/ip_fastfwd.c 22 May 2012 14:46:00 -0000
@@ -179,6 +179,7 @@
M_ASSERTVALID(m);
M_ASSERTPKTHDR(m);
+ ASSERT_NET_BYTE_ORDER(m);
bzero(&ro, sizeof(ro));
@@ -343,6 +344,7 @@
/*
* Convert to host representation
*/
+ ASSERT_NET_BYTE_ORDER(m);
ip->ip_len = ntohs(ip->ip_len);
ip->ip_off = ntohs(ip->ip_off);
@@ -361,6 +363,7 @@
M_ASSERTVALID(m);
M_ASSERTPKTHDR(m);
+ ASSERT_HOST_BYTE_ORDER(m);
ip = mtod(m, struct ip *); /* m may have changed by pfil hook */
dest.s_addr = ip->ip_dst.s_addr;
@@ -442,12 +445,14 @@
if (!PFIL_HOOKED(&V_inet_pfil_hook))
goto passout;
+ ASSERT_HOST_BYTE_ORDER(m);
if (pfil_run_hooks(&V_inet_pfil_hook, &m, ifp, PFIL_OUT, NULL) || m == NULL) {
goto drop;
}
M_ASSERTVALID(m);
M_ASSERTPKTHDR(m);
+ ASSERT_HOST_BYTE_ORDER(m);
ip = mtod(m, struct ip *);
dest.s_addr = ip->ip_dst.s_addr;
@@ -511,6 +516,7 @@
goto consumed;
}
+ ASSERT_HOST_BYTE_ORDER(m);
#ifndef ALTQ
/*
* Check if there is enough space in the interface queue
Index: sys/netinet/ip_icmp.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.145.2.2
diff -u -r1.145.2.2 ip_icmp.c
--- sys/netinet/ip_icmp.c 19 Mar 2012 20:49:16 -0000 1.145.2.2
+++ sys/netinet/ip_icmp.c 22 May 2012 14:31:17 -0000
@@ -185,6 +185,7 @@
unsigned icmplen, icmpelen, nlen;
KASSERT((u_int)type <= ICMP_MAXTYPE, ("%s: illegal ICMP type", __func__));
+ ASSERT_HOST_BYTE_ORDER(n);
#ifdef ICMPPRINTFS
if (icmpprintfs)
printf("icmp_error(%p, %x, %d)\n", oip, type, code);
@@ -336,6 +337,7 @@
void (*ctlfunc)(int, struct sockaddr *, void *);
int fibnum;
+ ASSERT_HOST_BYTE_ORDER(m);
/*
* Locate icmp structure in mbuf, and check
* that not corrupted and of at least minimum length.
@@ -866,6 +868,7 @@
register int hlen;
register struct icmp *icp;
+ ASSERT_HOST_BYTE_ORDER(m);
hlen = ip->ip_hl << 2;
m->m_data += hlen;
m->m_len -= hlen;
Index: sys/netinet/ip_input.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.393.2.3
diff -u -r1.393.2.3 ip_input.c
--- sys/netinet/ip_input.c 19 Mar 2012 20:49:16 -0000 1.393.2.3
+++ sys/netinet/ip_input.c 22 May 2012 14:23:45 -0000
@@ -385,6 +385,7 @@
struct in_addr odst; /* original dst address */
M_ASSERTPKTHDR(m);
+ ASSERT_NET_BYTE_ORDER(m);
if (m->m_flags & M_FASTFWD_OURS) {
/*
@@ -467,6 +468,7 @@
goto bad;
}
ip->ip_off = ntohs(ip->ip_off);
+ ASSERT_HOST_BYTE_ORDER(m);
/*
* Check that the amount of data in the buffers
@@ -1371,6 +1373,7 @@
struct route ro;
int error, type = 0, code = 0, mtu = 0;
+ ASSERT_HOST_BYTE_ORDER(m);
if (m->m_flags & (M_BCAST|M_MCAST) || in_canforward(ip->ip_dst) == 0) {
IPSTAT_INC(ips_cantforward);
m_freem(m);
Index: sys/netinet/ip_ipsec.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_ipsec.c,v
retrieving revision 1.28.2.1
diff -u -r1.28.2.1 ip_ipsec.c
--- sys/netinet/ip_ipsec.c 23 Sep 2011 00:51:37 -0000 1.28.2.1
+++ sys/netinet/ip_ipsec.c 22 May 2012 14:25:41 -0000
@@ -346,6 +346,7 @@
(*m)->m_pkthdr.csum_flags &= ~CSUM_SCTP;
}
#endif
+ ASSERT_HOST_BYTE_ORDER(*m);
ip->ip_len = htons(ip->ip_len);
ip->ip_off = htons(ip->ip_off);
Index: sys/netinet/ip_mroute.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.161.2.2
diff -u -r1.161.2.2 ip_mroute.c
--- sys/netinet/ip_mroute.c 28 Mar 2012 12:45:35 -0000 1.161.2.2
+++ sys/netinet/ip_mroute.c 22 May 2012 14:32:54 -0000
@@ -1496,6 +1496,7 @@
vifi_t vifi;
int plen = ip->ip_len;
+ ASSERT_HOST_BYTE_ORDER(m);
VIF_LOCK_ASSERT();
/*
@@ -2375,6 +2376,8 @@
struct mbuf *mb_copy = NULL;
int mtu;
+ ASSERT_HOST_BYTE_ORDER(m);
+
/* Take care of delayed checksums */
if (m->m_pkthdr.csum_flags & CSUM_DELAY_DATA) {
in_delayed_cksum(m);
Index: sys/netinet/ip_output.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.329.2.2
diff -u -r1.329.2.2 ip_output.c
--- sys/netinet/ip_output.c 10 Nov 2011 20:28:30 -0000 1.329.2.2
+++ sys/netinet/ip_output.c 22 May 2012 14:47:14 -0000
@@ -133,6 +133,7 @@
int no_route_but_check_spd = 0;
#endif
M_ASSERTPKTHDR(m);
+ ASSERT_HOST_BYTE_ORDER(m);
if (inp != NULL) {
INP_LOCK_ASSERT(inp);
@@ -434,6 +435,8 @@
}
}
+ ASSERT_HOST_BYTE_ORDER(m);
+
/*
* Verify that we have any chance at all of being able to queue the
* packet or packet fragments, unless ALTQ is enabled on the given
@@ -505,6 +508,7 @@
/* Run through list of hooks for output packets. */
odst.s_addr = ip->ip_dst.s_addr;
+ ASSERT_HOST_BYTE_ORDER(m);
error = pfil_run_hooks(&V_inet_pfil_hook, &m, ifp, PFIL_OUT, inp);
if (error != 0 || m == NULL)
goto done;
@@ -596,6 +600,7 @@
* If small enough for interface, or the interface will take
* care of the fragmentation for us, we can just send directly.
*/
+ ASSERT_HOST_BYTE_ORDER(m);
if (ip->ip_len <= mtu ||
(m->m_pkthdr.csum_flags & ifp->if_hwassist & CSUM_TSO) != 0 ||
((ip->ip_off & IP_DF) == 0 && (ifp->if_hwassist & CSUM_FRAGMENT))) {
@@ -628,6 +633,7 @@
* to avoid confusing lower layers.
*/
m->m_flags &= ~(M_PROTOFLAGS);
+ ASSERT_NET_BYTE_ORDER(m);
error = (*ifp->if_output)(ifp, m,
(struct sockaddr *)dst, ro);
goto done;
@@ -716,6 +722,8 @@
if (len < 8)
return EMSGSIZE;
+ ASSERT_HOST_BYTE_ORDER(m0);
+
/*
* If the interface will not calculate checksums on
* fragmented packets, then do it here.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120522150603.GF29536>