Subject: IP offset incorrect in ICMP errors.
To: None <tech-net@netbsd.org>
From: Darren Reed <darrenr@reed.wattle.id.au>
List: tech-net
Date: 12/06/1998 17:10:07
Reading some email on the IP Filter mailling list, someone mentioned that
the IP offset field in ICMP errors was erroneous - if it was converted into
host order, it never returned to network byte order. So I went looking and
sure enough, it is a correct analysis.
I've made a quick patch to fix this, and also convert some
"x = htons((u_int16_t)x)" statements into "HTONS(x)". I assume this is safe,
or if it's not, then perhaps the HTONS/NTOHS should be updated.
See below for patches.
Darren
p.s. there's also a KNF patch in there too :-)
*** ip_icmp.c.orig Sun Dec 6 17:04:21 1998
--- ip_icmp.c Sun Dec 6 17:03:52 1998
***************
*** 144,149 ****
--- 144,150 ----
*/
if (oip->ip_off &~ (IP_MF|IP_DF))
goto freeit;
+ HTONS(oip->ip_off);
if (oip->ip_p == IPPROTO_ICMP && type != ICMP_REDIRECT &&
n->m_len >= oiplen + ICMP_MINLEN &&
!ICMP_INFOTYPE(((struct icmp *)((caddr_t)oip + oiplen))->icmp_type)) {
*** ip_input.c.orig Sun Aug 9 21:11:14 1998
--- ip_input.c Sun Dec 6 17:03:08 1998
***************
*** 939,944 ****
--- 939,945 ----
}
return (0);
bad:
+ HTONS(ip->ip_id);
ip->ip_len -= ip->ip_hl << 2; /* XXX icmp_error adds in hdr length */
icmp_error(m, type, code, 0, 0);
ipstat.ips_badoptions++;
***************
*** 1186,1201 ****
if (rt->rt_ifa &&
(ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_subnetmask) ==
ifatoia(rt->rt_ifa)->ia_subnet) {
! if (rt->rt_flags & RTF_GATEWAY)
! dest = satosin(rt->rt_gateway)->sin_addr.s_addr;
! else
! dest = ip->ip_dst.s_addr;
! /* Router requirements says to only send host redirects */
! type = ICMP_REDIRECT;
! code = ICMP_REDIRECT_HOST;
#ifdef DIAGNOSTIC
! if (ipprintfs)
! printf("redirect (%d) to %x\n", code, (u_int32_t)dest);
#endif
}
}
--- 1187,1203 ----
if (rt->rt_ifa &&
(ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_subnetmask) ==
ifatoia(rt->rt_ifa)->ia_subnet) {
! if (rt->rt_flags & RTF_GATEWAY)
! dest = satosin(rt->rt_gateway)->sin_addr.s_addr;
! else
! dest = ip->ip_dst.s_addr;
! /* Router requirements says only send host redirects */
! type = ICMP_REDIRECT;
! code = ICMP_REDIRECT_HOST;
#ifdef DIAGNOSTIC
! if (ipprintfs)
! printf("redirect (%d) to %x\n", code,
! (u_int32_t)dest);
#endif
}
}
*** ip_output.c.orig Sun Aug 9 21:11:14 1998
--- ip_output.c Sun Dec 6 17:02:09 1998
***************
*** 368,375 ****
* If small enough for mtu of path, can just send directly.
*/
if ((u_int16_t)ip->ip_len <= mtu) {
! ip->ip_len = htons((u_int16_t)ip->ip_len);
! ip->ip_off = htons((u_int16_t)ip->ip_off);
ip->ip_sum = 0;
ip->ip_sum = in_cksum(m, hlen);
error = (*ifp->if_output)(ifp, m, sintosa(dst), ro->ro_rt);
--- 368,375 ----
* If small enough for mtu of path, can just send directly.
*/
if ((u_int16_t)ip->ip_len <= mtu) {
! HTONS(ip->ip_len);
! HTONS(ip->ip_off);
ip->ip_sum = 0;
ip->ip_sum = in_cksum(m, hlen);
error = (*ifp->if_output)(ifp, m, sintosa(dst), ro->ro_rt);
***************
*** 437,443 ****
}
m->m_pkthdr.len = mhlen + len;
m->m_pkthdr.rcvif = (struct ifnet *)0;
! mhip->ip_off = htons((u_int16_t)mhip->ip_off);
mhip->ip_sum = 0;
mhip->ip_sum = in_cksum(m, mhlen);
ipstat.ips_ofragments++;
--- 437,443 ----
}
m->m_pkthdr.len = mhlen + len;
m->m_pkthdr.rcvif = (struct ifnet *)0;
! HTONS(mhip->ip_off);
mhip->ip_sum = 0;
mhip->ip_sum = in_cksum(m, mhlen);
ipstat.ips_ofragments++;
***************
*** 451,457 ****
m_adj(m, hlen + firstlen - (u_int16_t)ip->ip_len);
m->m_pkthdr.len = hlen + firstlen;
ip->ip_len = htons((u_int16_t)m->m_pkthdr.len);
! ip->ip_off = htons((u_int16_t)(ip->ip_off | IP_MF));
ip->ip_sum = 0;
ip->ip_sum = in_cksum(m, hlen);
sendorfree:
--- 451,458 ----
m_adj(m, hlen + firstlen - (u_int16_t)ip->ip_len);
m->m_pkthdr.len = hlen + firstlen;
ip->ip_len = htons((u_int16_t)m->m_pkthdr.len);
! ip->ip_off |= IP_MF;
! HTONS(ip->ip_off);
ip->ip_sum = 0;
ip->ip_sum = in_cksum(m, hlen);
sendorfree:
***************
*** 1222,1229 ****
* than the interface's MTU. Can this possibly matter?
*/
ip = mtod(copym, struct ip *);
! ip->ip_len = htons((u_int16_t)ip->ip_len);
! ip->ip_off = htons((u_int16_t)ip->ip_off);
ip->ip_sum = 0;
ip->ip_sum = in_cksum(copym, ip->ip_hl << 2);
(void) looutput(ifp, copym, sintosa(dst), NULL);
--- 1223,1230 ----
* than the interface's MTU. Can this possibly matter?
*/
ip = mtod(copym, struct ip *);
! HTONS(ip->ip_len);
! HTONS(ip->ip_off);
ip->ip_sum = 0;
ip->ip_sum = in_cksum(copym, ip->ip_hl << 2);
(void) looutput(ifp, copym, sintosa(dst), NULL);