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);