Subject: ICMP error patch update.
To: None <tech-net@netbsd.org>
From: Darren Reed <darrenr@reed.wattle.id.au>
List: tech-net
Date: 12/06/1998 17:59:06
After further investigation, it appears that the IP packet length and ID
can also be in the wrong host order when ICMP errors are generated.  The
patches below also fix that problem (including previous ip_off patch).

Something which I believe is important is that this patch set introduce
an new `rule' when calling icmp_error(): ip_len must reflect the total
packet size.  Currently, before calling protocol handlers, ipintr()
adjusts ip_len to not include the IP header, so protocol input routines
will need to re-adjust before calling icmp_error() - hence a patch for
udp_usrrep.c is now included.  icmp_error() could attempt to detect being
called with an incorrect ip_len (is ip_hl<<2 > ip_len ?) but I don't think
it's a safe test to make.

Darren

p.s. this problem allegedly has its roots back in the original BSD4.4.

*** ip_icmp.c.orig	Sun Dec  6 17:04:21 1998
--- ip_icmp.c	Sun Dec  6 17:46:24 1998
***************
*** 159,165 ****
  	m = m_gethdr(M_DONTWAIT, MT_HEADER);
  	if (m == NULL)
  		goto freeit;
! 	icmplen = oiplen + min(8, oip->ip_len);
  	m->m_len = icmplen + ICMP_MINLEN;
  	MH_ALIGN(m, m->m_len);
  	icp = mtod(m, struct icmp *);
--- 159,165 ----
  	m = m_gethdr(M_DONTWAIT, MT_HEADER);
  	if (m == NULL)
  		goto freeit;
! 	icmplen = oiplen + min(8, oip->ip_len - oiplen);
  	m->m_len = icmplen + ICMP_MINLEN;
  	MH_ALIGN(m, m->m_len);
  	icp = mtod(m, struct icmp *);
***************
*** 183,188 ****
--- 183,191 ----
  			icp->icmp_nextmtu = htons(destifp->if_mtu);
  	}
  
+ 	HTONS(oip->ip_id);
+ 	HTONS(oip->ip_off);
+ 	HTONS(oip->ip_len);
  	icp->icmp_code = code;
  	bcopy((caddr_t)oip, (caddr_t)&icp->icmp_ip, icmplen);
  	nip = &icp->icmp_ip;
*** ip_input.c.orig	Sun Aug  9 21:11:14 1998
--- ip_input.c	Sun Dec  6 17:26:31 1998
***************
*** 1139,1145 ****
  		m_freem(m);
  		return;
  	}
- 	HTONS(ip->ip_id);
  	if (ip->ip_ttl <= IPTTLDEC) {
  		icmp_error(m, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, dest, 0);
  		return;
--- 1139,1144 ----
***************
*** 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
  		}
  	}
--- 1185,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 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:26:11 1998
***************
*** 172,177 ****
--- 172,178 ----
  		ipstat.ips_localout++;
  	} else {
  		hlen = ip->ip_hl << 2;
+ 		HTONS(ip->ip_id);
  	}
  	/*
  	 * Route packet.
***************
*** 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);
--- 369,376 ----
  	 * 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++;
--- 438,444 ----
  		}
  		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:
--- 452,459 ----
  	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);
--- 1224,1231 ----
  		 * 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);
*** udp_usrreq.c.orig	Wed Jan 14 01:41:37 1998
--- udp_usrreq.c	Sun Dec  6 17:44:53 1998
***************
*** 303,308 ****
--- 303,309 ----
  			/* It was a debugger connect packet, just drop it now */
  				goto bad;
  #endif
+ 			ip->ip_len += ip->ip_hl << 2;
  			icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_PORT, 0, 0);
  			return;
  		}