From owner-svn-src-head@FreeBSD.ORG Thu Apr 25 04:21:39 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 694676C9; Thu, 25 Apr 2013 04:21:39 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-ie0-x236.google.com (mail-ie0-x236.google.com [IPv6:2607:f8b0:4001:c03::236]) by mx1.freebsd.org (Postfix) with ESMTP id 221071E36; Thu, 25 Apr 2013 04:21:39 +0000 (UTC) Received: by mail-ie0-f182.google.com with SMTP id bn7so3081954ieb.41 for ; Wed, 24 Apr 2013 21:21:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=VEN8a7s5nOpRGEyaeoaG9IggzYurFNrNEBL5VqhQkF0=; b=j+IhorklmnIXd92VZXU9frgQVYBxfEPbkgiyBY2E5eSGxvacaicDvRqCrUraFBR7Hy XRYekvnQGMyH+u72awZif41TmdgzpyqK6qoKN39borDx+fib5CVG4NSWMJV0rNOsscm0 Jjj1YWKA/6SdlQSI2KKXR4LC1jMIH25KOkx4/vyElCLqhBX4W18wJ8tqlN66Hpgqqei/ kh9qL+OAYx1QaAAbDCnDDaRuahqUspgUkxm/+ucx9Ni5tFDb/7lE8jwJPxP1XX1rmXfc g08tRk5HvdXDDGswgrUf0/nzB34Lq834VHHOjzRNCGZfcIuGxs6BKc+WZb5/C3VbM0KU zQaA== X-Received: by 10.42.82.67 with SMTP id c3mr20476453icl.25.1366863698897; Wed, 24 Apr 2013 21:21:38 -0700 (PDT) Received: from gloom ([66.11.160.35]) by mx.google.com with ESMTPSA id xd4sm10457003igb.3.2013.04.24.21.21.37 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 24 Apr 2013 21:21:37 -0700 (PDT) Sender: Mark Johnston Date: Thu, 25 Apr 2013 00:21:31 -0400 From: Mark Johnston To: Randall Stewart Subject: Re: svn commit: r249848 - head/sys/netinet Message-ID: <20130425042131.GA3545@gloom> References: <201304241830.r3OIUWiZ073002@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201304241830.r3OIUWiZ073002@svn.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Apr 2013 04:21:39 -0000 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.