On Fri, Feb 22, 2008 at 10:04:12PM +0000, Bruce M. Simpson wrote:
> I looked at this very briefly.
> 
> It's gnarly because in_canforward() is a candidate for inlining and is a 
> predicate which is being overloaded with different meanings by 
> ip_forward()/ip_input() and icmp_reflect().
> 
> So whilst the fix is most likely a 3 liner, it risks making the code 
> look crap. We genuinely don't want to forward 169.254.0.0/16 traffic, 
> however we genuinely need to reply to ICMP which originates from these 
> ranges.

Attached is a patch that fixes the failure to respond to ICMP for
link-local addresses.  I reworked the opening if statement in
icmp_reflect() to make it a touch more readable, and eliminated the call
to in_canforward() since its logic doesn't work in this context.

Also, I took a cue from the IN_LINKLOCAL() macro and added two new
macros to sys/netinet/in.h to perform checks for the loopback network
and the "zero" network.  IN_LOOPBACK() and IN_ZERONET(), respectively.

IN_ZERONET seems like a lousy name, but the only alternative I could
find in RFC3330 was "'this' net," and IN_THISNET wasn't an improvement.


-Snow

diff -ru /usr/src/sys/netinet/in.h /usr/src.new/sys/netinet/in.h
--- /usr/src/sys/netinet/in.h	2007-06-12 16:24:53.000000000 +0000
+++ /usr/src.new/sys/netinet/in.h	2008-03-13 10:44:29.000000000 +0000
@@ -379,6 +379,8 @@
 #define	IN_BADCLASS(i)		(((u_int32_t)(i) & 0xf0000000) == 0xf0000000)
 
 #define IN_LINKLOCAL(i)		(((u_int32_t)(i) & 0xffff0000) == 0xa9fe0000)
+#define IN_LOOPBACK(i)		(((u_int32_t)(i) & 0xff000000) == 0x7f000000)
+#define IN_ZERONET(i)		(((u_int32_t)(i) & 0xff000000) == 0)
 
 #define	IN_PRIVATE(i)	((((u_int32_t)(i) & 0xff000000) == 0x0a000000) || \
 			 (((u_int32_t)(i) & 0xfff00000) == 0xac100000) || \
diff -ru /usr/src/sys/netinet/ip_icmp.c /usr/src.new/sys/netinet/ip_icmp.c
--- /usr/src/sys/netinet/ip_icmp.c	2007-10-07 20:44:23.000000000 +0000
+++ /usr/src.new/sys/netinet/ip_icmp.c	2008-03-13 11:03:44.000000000 +0000
@@ -622,13 +622,14 @@
 	struct mbuf *opts = 0;
 	int optlen = (ip->ip_hl << 2) - sizeof(struct ip);
 
-	if (!in_canforward(ip->ip_src) &&
-	    ((ntohl(ip->ip_src.s_addr) & IN_CLASSA_NET) !=
-	     (IN_LOOPBACKNET << IN_CLASSA_NSHIFT))) {
+	if (IN_MULTICAST(ntohl(ip->ip_src.s_addr)) ||
+	    IN_EXPERIMENTAL(ntohl(ip->ip_src.s_addr)) ||
+	    IN_ZERONET(ntohl(ip->ip_src.s_addr)) ) {
 		m_freem(m);	/* Bad return address */
 		icmpstat.icps_badaddr++;
 		goto done;	/* Ip_output() will check for broadcast */
 	}
+
 	t = ip->ip_dst;
 	ip->ip_dst = ip->ip_src;
 
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to