Re: kern/61215: off-by-one error likely in ip_fragment()

David,

the problem with if_gre is actually twofold:

  - the change of htons(m->m_pkthdr.len) in the last commit to that
    file is incorrect.  In FreeBSD this is done in ip_output for all
    packets sent (unless RAW).

  - The struct ip which is contained in struct gh is not correctly
    intialized.  For some reason this didn't matter until now.  It seems
    M_PREPREND may return non-zeroed memory.

There is no problem in either ip_fragment() nor m_copym() (and the
'fix' I posted is bogus, however some of those KASSERTs are highly
bogus too and misleading).

Please try the attached patch.  I was able to get correct GRE packets
with that patch (as seen by ethereal).

I'm not sure if it is better to do a bzero() on the entire struct gh
to have all ip header values set to zero for sure.  There are still
some that are unitialized.

-- 
Andre


Index: if_gre.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_gre.c,v
retrieving revision 1.17
diff -u -p -r1.17 if_gre.c
--- if_gre.c	30 Dec 2003 11:41:42 -0000	1.17
+++ if_gre.c	14 Jan 2004 14:40:09 -0000
_at__at_ -341,7 +341,7 _at__at_ gre_output(struct ifnet *ifp, struct mbu
  		goto end;
  	}

-	if (m == NULL) {	/* impossible */
+	if (m == NULL) {	/* mbuf allocation failed */
  		_IF_DROP(&ifp->if_snd);
  		error = ENOBUFS;
  		goto end;
_at__at_ -363,13 +363,14 _at__at_ gre_output(struct ifnet *ifp, struct mbu
  		((struct ip*)gh)->ip_ttl = GRE_TTL;
  		((struct ip*)gh)->ip_tos = ip->ip_tos;
  		((struct ip*)gh)->ip_id = ip->ip_id;
-		gh->gi_len = htons(m->m_pkthdr.len);
+		((struct ip*)gh)->ip_off = 0;
+		gh->gi_len = m->m_pkthdr.len;
  	}

  	ifp->if_opackets++;
  	ifp->if_obytes += m->m_pkthdr.len;
  	/* send it off */
-	error = ip_output(m, NULL, &sc->route, 0,
+	error = ip_output(m, NULL, &sc->route, IP_FORWARDING,
  	    (struct ip_moptions *)NULL, (struct inpcb *)NULL);
    end:
  	sc->called = 0;