divert_output() has a basic sanity check to ensure that the m_pkthdr.len
for reinjected packets is not shorter than the minimum length based on
the protocol:

        if (p_hdrlen && m->m_pkthdr.len < off + p_hdrlen)
                goto fail;

off is the length of the IP header, and p_hdrlen is the expected length
of the protocol header, e.g. sizeof(struct tcphdr) for TCP.

However, this check is currently wrong for ICMP in certain cases,
because sizeof(struct icmp) is 28 but an ICMP header can be as small as
8 bytes, e.g. an ICMP echo request packet with no payload.  So this
causes divert(4) to incorrectly discard ICMP packets with a payload
length of 0-19 bytes.

To fix this, I would like to change the check for ICMP from
sizeof(struct icmp) to ICMP_MINLEN, which according to netinet/ip_icmp.h
is the absolute minimum length of an ICMP packet (8 bytes).

I would also like to rename the p_hdrlen variable to min_hdrlen to
better reflect its purpose.

Although ICMPv6 is not affected, this diff renames p_hdrlen in
divert6_output() to min_hdrlen as well to be consistent with
divert_output().

ok?


Index: netinet/ip_divert.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.26
diff -u -p -r1.26 ip_divert.c
--- netinet/ip_divert.c 12 Jul 2014 19:05:45 -0000      1.26
+++ netinet/ip_divert.c 13 Jul 2014 03:46:20 -0000
@@ -85,7 +85,7 @@ divert_output(struct inpcb *inp, struct 
        struct sockaddr_in *sin;
        struct socket *so;
        struct ifaddr *ifa;
-       int s, error = 0, p_hdrlen = 0, dir;
+       int s, error = 0, min_hdrlen = 0, dir;
        struct ip *ip;
        u_int16_t off;
 
@@ -119,22 +119,22 @@ divert_output(struct inpcb *inp, struct 
 
        switch (ip->ip_p) {
        case IPPROTO_TCP:
-               p_hdrlen = sizeof(struct tcphdr);
+               min_hdrlen = sizeof(struct tcphdr);
                m->m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
                break;
        case IPPROTO_UDP:
-               p_hdrlen = sizeof(struct udphdr);
+               min_hdrlen = sizeof(struct udphdr);
                m->m_pkthdr.csum_flags |= M_UDP_CSUM_OUT;
                break;
        case IPPROTO_ICMP:
-               p_hdrlen = sizeof(struct icmp);
+               min_hdrlen = ICMP_MINLEN;
                m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
                break;
        default:
                /* nothing */
                break;
        }
-       if (p_hdrlen && m->m_pkthdr.len < off + p_hdrlen)
+       if (min_hdrlen && m->m_pkthdr.len < off + min_hdrlen)
                goto fail;
 
        m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
Index: netinet6/ip6_divert.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.27
diff -u -p -r1.27 ip6_divert.c
--- netinet6/ip6_divert.c       12 Jul 2014 19:05:45 -0000      1.27
+++ netinet6/ip6_divert.c       13 Jul 2014 03:46:20 -0000
@@ -89,7 +89,7 @@ divert6_output(struct inpcb *inp, struct
        struct sockaddr_in6 *sin6;
        struct socket *so;
        struct ifaddr *ifa;
-       int s, error = 0, p_hdrlen = 0, nxt = 0, off, dir;
+       int s, error = 0, min_hdrlen = 0, nxt = 0, off, dir;
        struct ip6_hdr *ip6;
 
        m->m_pkthdr.rcvif = NULL;
@@ -128,22 +128,22 @@ divert6_output(struct inpcb *inp, struct
 
        switch (nxt) {
        case IPPROTO_TCP:
-               p_hdrlen = sizeof(struct tcphdr);
+               min_hdrlen = sizeof(struct tcphdr);
                m->m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
                break;
        case IPPROTO_UDP:
-               p_hdrlen = sizeof(struct udphdr);
+               min_hdrlen = sizeof(struct udphdr);
                m->m_pkthdr.csum_flags |= M_UDP_CSUM_OUT;
                break;
        case IPPROTO_ICMPV6:
-               p_hdrlen = sizeof(struct icmp6_hdr);
+               min_hdrlen = sizeof(struct icmp6_hdr);
                m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
                break;
        default:
                /* nothing */
                break;
        }
-       if (p_hdrlen && m->m_pkthdr.len < off + p_hdrlen)
+       if (min_hdrlen && m->m_pkthdr.len < off + min_hdrlen)
                goto fail;
 
        m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;

Reply via email to