Subject: Re: kern/7818: ICMP protocol packets are not handled for localhost
To: Darren Reed <darrenr@reed.wattle.id.au>
From: Jun Xie <jun@qnx.com>
List: tech-net
Date: 06/21/1999 12:13:26
> Folks,
>       I've submitted a PR on bad ICMP behaviour and in researching a fix,
> found that the bug which causes the "unknown protocol" counter to get
> increased is what (on BSD) makes the UDP traceroute possible.  The bug
> appears to be that in ip_icmp.c, at about line 363 (in the code section
> after the "deliver" label) there's a "break" rather than a "return".  The
> subsequent effect is to let the packet drop through to the "raw" label
> and get fed into rip_input() and hence traceroute is able to get it.  I'm
> sure there's a good reason to pass processed packets on to rip_input()
> that have already been processed...I'm just not sure I understand what
> that would be and if so, why it would be limited to ICMP (other than to
> support traceroute).  Anyway, the easiest fix I can think of which won't
> disturb anything (except for correcting the "unknown protocol" statistics
> count) is to add another mbuf flag - M_DELIVERED (0x800) - set that in
> icmp_input() before calling rip_input() and not fiddle with the statistics
> counters in the tail end of rip_input() if M_DELIVERED is set.  If this
> is an acceptable patch, I'd like for it to be pulled up into 1.4.1.

This problem is mentioned in R. Stevens's TCP/IP Illustrated Vol. 2
(P.1055 - P.1056). Exercise 32.4 even gives a hint on how to fix it,
although the fix is used to add ICMP_UNREACH_PROTOCOL support.

I think the bug was introduced when BSD4.3 was upgraded to BSD4.4.
In BSD4.3, icmp_input() calls raw_input() to deliver some ICMP messages
to raw listeners. So ips_noproto does not get incremented. But in
BSD4.4, icmp_input() calls rip_input() instead. Thus the problem.

A possible fix is to replace

	m_freem(m);
	ipstat.ips_noproto++;
	ipstat.ips_delivered--;

with

	if (inetsw[ip_protox[ip->ip_p]].pr_input != rip_input)
		m_freem(m);
	else {
		icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_PROTOCOL, 0, 0);
		ipstat.ips_noproto++;
	}
	ipstat.ips_delivered--;

in rip_input().

This way, M_DELIVERED does not need to be defined.
If you insist on M_DELIVERED, you have to add

	m->m_flags |= M_DELIVERED;

in igmp_input(), because it calls rip_input() to pass all valid IGMP
packets to raw IGMP sockets. Otherwise, if there is no raw IGMP
listeners, ips_noproto is incremented too.

Jun