Subject: Re: bpf breakage
To: None <tech-net@NetBSD.org>
From: David Young <dyoung@pobox.com>
List: tech-net
Date: 07/02/2004 23:01:59
On Sat, Jul 03, 2004 at 12:43:49PM +0900, itojun@iijlab.net wrote:
> >On Sat, Jul 03, 2004 at 10:40:54AM +0900, itojun@iijlab.net wrote:
> >> 	due to this change in sys/net/bpf.c (bpf_measure -> m_length),
> >> 	bpf on many interfaces (which prepends address family by integer)
> >> 	is broken.  the length will become off by 4, and you'll see
> >> 	"truncated-ip" message from tcpdump.  please back it out.
> >> 
> >> 	my guess is the culprit is "optimize m_length" (2nd paragraph from the
> >> 	bottom).
> >
> >On which interfaces is it broken, Itojun?  Why do those interfaces
> >assemble packets with incorrect m_pkthdr.len?
> 
> 	i got a breakage report on ppp interface (but the originator mentioned
> 	that he would like to remain anonymous, so i don't cc: him).
> #	i've forwarded those to you privately.
> 	some of the interfaces (like loopback) prepends 4byte mbuf (address
> 	family is set inside) on bpf_mtap.  in such case, m_pkthdr.len
> 	will not be useful.

Are other interfaces affected?

This looks to me like a latent ppp bug.  At sys/net/if_ppp.c:874,
M_PREPEND functionality is incompletely duplicated.  Compare this

    /*      
     * Add PPP header.  If no space in first mbuf, allocate another.
     * (This assumes M_LEADINGSPACE is always 0 for a cluster mbuf.)
     */
    if (M_LEADINGSPACE(m0) < PPP_HDRLEN) {
        m0 = m_prepend(m0, PPP_HDRLEN, M_DONTWAIT);
        if (m0 == 0) {
            error = ENOBUFS;
            goto bad;
        }                 
        m0->m_len = 0;
    } else
        m0->m_data -= PPP_HDRLEN;
         
    cp = mtod(m0, u_char *);
    *cp++ = address;
    *cp++ = control; 
    *cp++ = protocol >> 8;
    *cp++ = protocol & 0xff;
    m0->m_len += PPP_HDRLEN;

with this

	#define M_PREPEND(m, plen, how)                                         \
	do {                                                                    \
			if (M_LEADINGSPACE(m) >= (plen)) {                              \
					(m)->m_data -= (plen);                                  \
					(m)->m_len += (plen);                                   \
			} else                                                          \
					(m) = m_prepend((m), (plen), (how));                    \
			if ((m) && (m)->m_flags & M_PKTHDR)                             \
					(m)->m_pkthdr.len += (plen);                            \
	} while (/* CONSTCOND */ 0)

The difference is that M_PREPEND updates m_pkthdr.len, but ppp does not.

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933