[net.bugs.4bsd] two bug reports on ip_icmp.c

mogul@Gregorio.ARPA (11/27/84)

Subject: icmp_input() improperly handles packets in multiple mbufs

Index:	sys/netinet/ip_icmp.c 4.2BSD Fix

Description:
	If an ICMP packet "arrives" as 2 mbufs, it may get interpreted
	improperly.
Repeat-By:
	Send a short ICMP (e.g., an ECHOME with no additional data)
	to "localhost" or to your own 10Mbit ethernet address.  It
	will not be answered, and will probably show up in netstat -s
	reports as an ICMP type code #0.
Fix:
	icmp_input() calls m_pullup() to make sure that the ICMP
	header is in a contiguous buffer.  The calculation for how
	long the buffer must be should include the the IP header, but
	because ip->ip_len has been adjusted in ip_intr() to not count
	the header length, the call to m_pullup() is given a short
	length.

	A diff -c listing shows the change.  WARNING: this is not the
	first bug fix to this piece of code; the original 4.2 distribution
	had far more serious problems here.
***************
*** 129,135
  		goto free;
  	}
  	/* THIS LENGTH CHECK STILL MISSES ANY IP OPTIONS IN ICMP_IP */
! 	i = MIN(icmplen, ICMP_ADVLENMIN + hlen);
   	if ((m->m_off > MMAXOFF || m->m_len < i) &&
   		(m = m_pullup(m, i)) == 0)  {
  		icmpstat.icps_tooshort++;

--- 134,140 -----
  		goto free;
  	}
  	/* THIS LENGTH CHECK STILL MISSES ANY IP OPTIONS IN ICMP_IP */
! 	i = MIN(icmplen + hlen, ICMP_ADVLENMIN + hlen);
   	if ((m->m_off > MMAXOFF || m->m_len < i) &&
   		(m = m_pullup(m, i)) == 0)  {
  		icmpstat.icps_tooshort++;

Subject: icmp output statistics don't record replies

Index:	sys/netinet/ip_icmp.c 4.2BSD Fix

Description:
	ICMP packets sent by the kernel in response to incoming ICMP
	packets (e.g., ECHOREPLYs) are not recorded in the IMCP
	statistics block.
Repeat-By:
	Send an ICMP ECHO to your host; run netstat -s and observe
	that the "input histogram" registers the request but the
	"output histogram" doesn't register the response.
Fix:
	Add one line of code (your line numbers may vary):
***************
*** 256,261
  		goto free;
  	}
  reflect:
  	ip->ip_len += hlen;		/* since ip_input deducts this */
  	icmpstat.icps_reflect++;
  	icmp_reflect(ip);

--- 299,305 -----
  		goto free;
  	}
  reflect:
+ 	icmpstat.icps_outhist[icp->icmp_type]++;
  	ip->ip_len += hlen;		/* since ip_input deducts this */
  	icmpstat.icps_reflect++;
  	icmp_reflect(ip);