Bug 61215
| Summary: | off-by-one error likely in ip_fragment() | ||
|---|---|---|---|
| Product: | Base System | Reporter: | David Gilbert <dgilbert> |
| Component: | kern | Assignee: | Andre Oppermann <andre> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | 5.2-CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
Responsible Changed From-To: freebsd-bugs->andre Take over. 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
@@ -341,7 +341,7 @@ 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;
@@ -363,13 +363,14 @@ 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;
State Changed From-To: open->analyzed The bug is in if_gre.c, not ip_fragment or m_copym. >>>>> "Andre" == Andre Oppermann <andre@freebsd.org> writes: Andre> David, the problem with if_gre is actually twofold: Andre> - the change of htons(m->m_pkthdr.len) in the last commit to Andre> that file is incorrect. In FreeBSD this is done in ip_output Andre> for all packets sent (unless RAW). Andre> - The struct ip which is contained in struct gh is not Andre> correctly intialized. For some reason this didn't matter until Andre> now. It seems M_PREPREND may return non-zeroed memory. Andre> There is no problem in either ip_fragment() nor m_copym() (and Andre> the 'fix' I posted is bogus, however some of those KASSERTs are Andre> highly bogus too and misleading). Andre> Please try the attached patch. I was able to get correct GRE Andre> packets with that patch (as seen by ethereal). Andre> I'm not sure if it is better to do a bzero() on the entire Andre> struct gh to have all ip header values set to zero for sure. Andre> There are still some that are unitialized. I'm not sure what's up. Your patch wouldn't apply to v1.17 of my if_gre.c, so something's wrong with the patch. Regardless, I applied the patch by hand and things didn't work yet. The kernel didn't crash, but packets routed into the tunnel didn't show up on the outbound interface. I my case, the machine has three ethernet-like interfaces and the gre. wi0 and sis0 are internal networks. dc0 is the external network interface. A /32 route for the far end of the tunnel exists (and works on the new kernel ... it pings), but pings into the tunnel don't generate traffic on dc0 (at least according to tcpdump). Dave. -- ============================================================================ |David Gilbert, Independent Contractor. | Two things can only be | |Mail: dave@daveg.ca | equal if and only if they | |http://daveg.ca | are precisely opposite. | =========================================================GLO================ David Gilbert wrote: > > >>>>> "Andre" == Andre Oppermann <andre@freebsd.org> writes: > > Andre> David, the problem with if_gre is actually twofold: > > Andre> - the change of htons(m->m_pkthdr.len) in the last commit to > Andre> that file is incorrect. In FreeBSD this is done in ip_output > Andre> for all packets sent (unless RAW). > > Andre> - The struct ip which is contained in struct gh is not > Andre> correctly intialized. For some reason this didn't matter until > Andre> now. It seems M_PREPREND may return non-zeroed memory. > > Andre> There is no problem in either ip_fragment() nor m_copym() (and > Andre> the 'fix' I posted is bogus, however some of those KASSERTs are > Andre> highly bogus too and misleading). > > Andre> Please try the attached patch. I was able to get correct GRE > Andre> packets with that patch (as seen by ethereal). > > Andre> I'm not sure if it is better to do a bzero() on the entire > Andre> struct gh to have all ip header values set to zero for sure. > Andre> There are still some that are unitialized. > > I'm not sure what's up. Your patch wouldn't apply to v1.17 of my > if_gre.c, so something's wrong with the patch. Regardless, I applied > the patch by hand and things didn't work yet. Didn't it apply because of patch complaining or because it didn't match at all? > The kernel didn't crash, but packets routed into the tunnel didn't > show up on the outbound interface. I my case, the machine has three > ethernet-like interfaces and the gre. > > wi0 and sis0 are internal networks. dc0 is the external network > interface. A /32 route for the far end of the tunnel exists (and > works on the new kernel ... it pings), but pings into the tunnel don't > generate traffic on dc0 (at least according to tcpdump). Do you enable "link1" on your GRE interface? What does ifconfig -a show? -- Andre >>>>> "Andre" == Andre Oppermann <andre@freebsd.org> writes: >> I'm not sure what's up. Your patch wouldn't apply to v1.17 of my >> if_gre.c, so something's wrong with the patch. Regardless, I >> applied the patch by hand and things didn't work yet. Andre> Didn't it apply because of patch complaining or because it Andre> didn't match at all? It looked like it was mostly whitespace errors or somesuch. The patch algorythm couldn't find the are to patch. ... however, as I said... I was able to patch it by hand. >> The kernel didn't crash, but packets routed into the tunnel didn't >> show up on the outbound interface. I my case, the machine has >> three ethernet-like interfaces and the gre. >> >> wi0 and sis0 are internal networks. dc0 is the external network >> interface. A /32 route for the far end of the tunnel exists (and >> works on the new kernel ... it pings), but pings into the tunnel >> don't generate traffic on dc0 (at least according to tcpdump). Andre> Do you enable "link1" on your GRE interface? What does link1 do to gre? It hasn't been necessary before. Andre> What does ifconfig -a show? Ifconfig -a output was normal. I didn't save the new output, but the old output is the same ... save the fact that the new output prints the tunnel endpoints. gre0: flags=9011<UP,POINTOPOINT,LINK0,MULTICAST> mtu 1476 inet6 fe80::2d0:9ff:fee4:bbc2%gre0 prefixlen 64 scopeid 0x5 inet 66.246.133.114 --> 66.246.133.113 netmask 0xfffffffc Dave. -- ============================================================================ |David Gilbert, Independent Contractor. | Two things can only be | |Mail: dave@daveg.ca | equal if and only if they | |http://daveg.ca | are precisely opposite. | =========================================================GLO================ Whatever's checked into -CURRENT appears to be correct now. State Changed From-To: analyzed->closed Closed at submitter's request |
It would appear that GRE calling ip_fragment() is leading to an an immediate crash. The machine in question crashes dependably during boot. The following is the backtrace: panic messages: --- panic: m_copym, offset > size of mbuf chain #0 doadump () at /usr/src/sys/kern/kern_shutdown.c:240 #1 0xc0508512 in boot (howto=256) at /usr/src/sys/kern/kern_shutdown.c:372 #2 0xc0508868 in panic () at /usr/src/sys/kern/kern_shutdown.c:550 #3 0xc0544fa5 in m_copym (m=0x0, off0=1500, len=1480, wait=4) at /usr/src/sys/kern/uipc_mbuf.c:211 #4 0xc059b941 in ip_fragment (ip=0xc1e919e8, m_frag=0xdf92c9e0, mtu=-1041688000, if_hwassist_flags=0, sw_csum=1) at /usr/src/sys/netinet/ip_output.c:1219 #5 0xc059b55f in ip_output (m0=0x1, opt=0xc1e919e8, ro=0xc5f8edfc, flags=0, imo=0x0, inp=0x0) at /usr/src/sys/netinet/ip_output.c:1047 #6 0xc611054f in gre_output (ifp=0xc5f8ec00, m=0xc1e91900, dst=0xc1e919e8, rt=0xc612ce00) at /usr/src/sys/net/if_gre.c:372 #7 0xc059b4f0 in ip_output (m0=0x1, opt=0xc2b2a00e, ro=0xdf92cb7c, flags=1, imo=0x0, inp=0x0) at /usr/src/sys/netinet/ip_output.c:1021 #8 0xc059a3c6 in ip_forward (m=0xc1e8bb00, srcrt=0, next_hop=0x0) at /usr/src/sys/netinet/ip_input.c:1929 #9 0xc0598db0 in ip_input (m=0xc1e8bb00) at /usr/src/sys/netinet/ip_input.c:739 #10 0xc057bc7e in netisr_processqueue (ni=0xc074a718) at /usr/src/sys/net/netisr.c:152 #11 0xc057c093 in swi_net (dummy=0x0) at /usr/src/sys/net/netisr.c:257 #12 0xc04f5112 in ithread_loop (arg=0xc1e74500) at /usr/src/sys/kern/kern_intr.c:544 #13 0xc04f4104 in fork_exit (callout=0xc04f4f80 <ithread_loop>, arg=0x0, frame=0x0) at /usr/src/sys/kern/kern_fork.c:796 Fix: None yet. I have some thoughts. It looks like there's some stack corruption. in frame 4, mtu=-1041688000 and m = 0x0 in frame 3. Some values make sense and others do not. Dave. How-To-Repeat: configure an if_gre tunnel over an ethernet link. I havn't confirmed yet whether the cause depends on the machine in question being a router ... but it would most certainly influence it.