Subject: Re: bpf breakage
To: None <tech-net@NetBSD.org>
From: David Young <dyoung@pobox.com>
List: tech-net
Date: 07/02/2004 23:20:00
--82I3+IH0IqGh5yIs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Jul 03, 2004 at 01:08:15PM +0900, itojun@iijlab.net wrote:
> 
> 
> >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
> >
> >The difference is that M_PREPEND updates m_pkthdr.len, but ppp does not.
> 
> 	hmm, i guess you right...  are there any secret reason why ppp does
> 	not update m_pkthdr.len? >ppp guru

It is strange, isn't it?  Probably just an oversight.  Attached is a
patch for the anonymous PR sender to try.  (I have not even tried to
compile it yet.)  I will look again at this tomorrow morning....

Dave

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

--82I3+IH0IqGh5yIs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=ppp-patch

Index: if_ppp.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ppp.c,v
retrieving revision 1.89
diff -u -r1.89 if_ppp.c
--- if_ppp.c	21 Apr 2004 18:40:39 -0000	1.89
+++ if_ppp.c	3 Jul 2004 04:14:47 -0000
@@ -872,29 +872,21 @@
     }
 
     /*
-     * Add PPP header.  If no space in first mbuf, allocate another.
-     * (This assumes M_LEADINGSPACE is always 0 for a cluster mbuf.)
+     * Add PPP header.
      */
-    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;
+    M_PREPEND(m0, PPP_HDRLEN, M_DONTWAIT);
+    if (m0 == NULL) {
+	error = ENOBUFS;
+	goto bad;
+    }
 
     cp = mtod(m0, u_char *);
     *cp++ = address;
     *cp++ = control;
     *cp++ = protocol >> 8;
     *cp++ = protocol & 0xff;
-    m0->m_len += PPP_HDRLEN;
 
-    len = 0;
-    for (m = m0; m != 0; m = m->m_next)
-	len += m->m_len;
+    len = m_length(m0);
 
     if (sc->sc_flags & SC_LOG_OUTPKT) {
 	printf("%s output: ", ifp->if_xname);

--82I3+IH0IqGh5yIs--