kern/168190: [pf] panic when using pf and route-to (maybe: bad fragment handling?)

Ermal Luçi eri at freebsd.org
Tue May 22 18:09:52 UTC 2012


iirc this is from fastforwarding being enabled.
Just from memory though, cause i remember seeing this panic as well.

Again, from memory this is fastforwarding related, try disabling it.
If it was pf(4) surely in pfSense would have been seen more frequently
and in pfSense fastforwarding is not used but normal path....

On Tue, May 22, 2012 at 11:06 AM, Daniel Hartmeier <daniel at benzedrine.cx> wrote:
> 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.
> _______________________________________________
> freebsd-pf at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-pf
> To unsubscribe, send any mail to "freebsd-pf-unsubscribe at freebsd.org"



-- 
Ermal


More information about the freebsd-pf mailing list