svn commit: r249848 - head/sys/netinet

[Mark Johnston]

On Wed, Apr 24, 2013 at 06:30:32PM +0000, Randall Stewart wrote:
> Author: rrs
> Date: Wed Apr 24 18:30:32 2013
> New Revision: 249848
> URL: http://svnweb.freebsd.org/changeset/base/249848
> 
> Log:
>   This fixes the issue with the "randomly changing" default
>   route. What it was is there are two places in ip_output.c
>   where we do a goto again. One place was fine, it
>   copies out the new address and then resets dst = ro->rt_dst;
>   But the other place does *not* do that, which means earlier
>   when we found the gateway, we have dst pointing there
>   aka dst = ro->rt_gateway is done.. then we do a
>   goto again.. bam now we clobber the default route.
>   
>   The fix is just to move the again so we are always
>   doing dst = &ro->rt_dst; in the again loop.

Hi Randall,

>From my reading of ip6_output() it seems as though a similar bug exists
in the IPv6 case. It's not as bad though: the clobbering write to *dst
only happens if the route lookup of the rewritten destination address
doesn't return anything. So we'd need to have an IPv6 packet match a
gateway route and then be diverted by a pfil hook to a destination for
which selectroute() returns a NULL route. In this case we'll see the
same problem as in the IPv4 case - the original gateway route will be
clobbered.

Would you (or anyone familiar with the code in question) be able to take
a look and confirm whether this is the case? The fix would be the same I
think (patch below).

Thanks!
-Mark

Index: ip6_output.c
===================================================================
--- ip6_output.c	(revision 249866)
+++ ip6_output.c	(working copy)
@@ -519,7 +519,6 @@
 	ro_pmtu = ro;
 	if (opt && opt->ip6po_rthdr)
 		ro = &opt->ip6po_route;
-	dst = (struct sockaddr_in6 *)&ro->ro_dst;
 #ifdef FLOWTABLE
 	if (ro->ro_rt == NULL) {
 		struct flentry *fle;
@@ -536,6 +535,8 @@
 	}
 #endif
 again:
+	dst = (struct sockaddr_in6 *)&ro->ro_dst;
+
 	/*
 	 * if specified, try to fill in the traffic class field.
 	 * do not override if a non-zero value is already set.