Subject: Re: kern/4827
To: Joel Lindholm <joel@effnet.se>
From: Jun-ichiro itojun Hagino <itojun@iijlab.net>
List: tech-net
Date: 02/07/2001 01:15:59
>	http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=4827
>	The problem still exists (not sure if the kernel panics, but we can
>	have bogus routing table entry easily).  The step to repeat
>	the problem is like this:
(snip)

	the following (simple and full-of-comment) patch avoids any
	route update/creation in rtredirect(), if rt_key == rt_gateway.

>	now, what should we do?
>	1. ignore icmp redirect if we have RTF_HOST|RTF_STATIC entry.
>	   it will not cause the entry to get updated, and can cause
>	   lots of icmp redirects.  but since it is the root user who asked
>	   for the entry, maybe the kernel shouldn't modify it.

	it seemed not enough, since:
	# ifconfig if0 10.0.0.1 netmask 0xffffff00
	# route add default 10.0.0.2
	# route add 10.0.0.128 netmask 0xffffff80	(bogus route!)
	# ping 10.0.0.129
	will lead us to a new routing entry 10.0.0.129 -> 10.0.0.129.

itojun


Index: netbsd/sys/net/route.c
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/net/route.c,v
retrieving revision 1.17
retrieving revision 1.19
diff -u -r1.17 -r1.19
--- netbsd/sys/net/route.c	2001/01/27 18:17:07	1.17
+++ netbsd/sys/net/route.c	2001/02/06 16:12:22	1.19
@@ -328,6 +328,18 @@
 	if (rt->rt_flags & RTF_GATEWAY) {
 		if (((rt->rt_flags & RTF_HOST) == 0) && (flags & RTF_HOST)) {
 			/*
+			 * Don't create entry if we are going to set
+			 * rt_key and rt_gateway to the same value.  It will
+			 * only happen when we have manually configured
+			 * network route toward part of onlink destinations
+			 * (is bogus with IPv4/v6).
+			 * XXX non-IP protocol needs checking
+			 */
+			if (equal(dst, gateway)) {
+				error = EINVAL;
+				goto done;
+			}
+			/*
 			 * Changing from route to net => route to host.
 			 * Create new route, rather than smashing route to net.
 			 */
@@ -346,6 +358,25 @@
 				flags = rt->rt_flags;
 			stat = &rtstat.rts_dynamic;
 		} else {
+			/*
+			 * Ditto (see above), when we update an entry.
+			 * This happens only with manually configured host
+			 * route toward an onlink destination.
+			 *
+			 * In this case, we may be able to nuke the entry
+			 * and then overwrite it with cloned RTF_LLINFO route.
+			 * We don't do that at this moment since (1) we are
+			 * not sure if RTF_CLONING interface route exists,
+			 * (2) there's no case where RTF_STATIC gets erased
+			 * or overwritten by non-static routes from within
+			 * the kernel, and (3) the analysis assumes too much
+			 * about IPv4/v6-like L3 behavior.
+			 * XXX non-IP protocol needs checking
+			 */
+			if (equal(rt_key(rt), gateway)) {
+				error = EINVAL;
+				goto done;
+			}
 			/*
 			 * Smash the current notion of the gateway to
 			 * this destination.  Should check about netmask!!!