NetBSD Problem Report #17202

Received: (qmail 16337 invoked by uid 605); 10 Jun 2002 02:53:24 -0000
Message-Id: <20020610025252.1043A7BA@starfruit.itojun.org>
Date: Mon, 10 Jun 2002 11:52:52 +0900 (JST)
From: itojun@itojun.org
Sender: gnats-bugs-owner@netbsd.org
Reply-To: itojun@itojun.org
To: gnats-bugs@gnats.netbsd.org
Subject: SIOCSIFADDR/IFDSTADDR on gre* interface destroys outer IP address setting
X-Send-Pr-Version: 3.95

>Number:         17202
>Category:       kern
>Synopsis:       SIOCSIFADDR/IFDSTADDR on gre* interface destroys outer IP address setting
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 10 02:54:00 +0000 2002
>Closed-Date:    
>Last-Modified:  Mon Jun 10 20:10:01 +0000 2002
>Originator:     Jun-ichiro itojun Hagino
>Release:        NetBSD 1.6A
>Organization:
	itojun
>Environment:
System: NetBSD starfruit.itojun.org 1.6A NetBSD 1.6A (STARFRUIT) #60: Mon Jun 10 02:00:55 JST 2002 itojun@starfruit.itojun.org:/usr/home/itojun/NetBSD/src/sys/arch/i386/compile/STARFRUIT i386
Architecture: i386
Machine: i386
>Description:
	gre* interface uses addresses configured by SIOCSIFADDR/IFDSTADDR to
	be IPv4 outer address pair (set by SIOCS*PHYADDR).  SIOCSIFADDR/
	IFDSTADDR operation overwrites SIOCS*PHYADDR setting, and the behavior
	is not intuitive.
>How-To-Repeat:
	code inspection
>Fix:
	remove SIOCSIFADDR/IFDSTADDR handling code from sys/net/if_gre.c.
	issue - backward compatibility.

Index: if_gre.c
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_gre.c,v
retrieving revision 1.35
diff -u -r1.35 if_gre.c
--- if_gre.c	2002/06/09 19:17:43	1.35
+++ if_gre.c	2002/06/10 02:53:09
@@ -356,26 +356,9 @@
 	s = splnet();
 	switch (cmd) {
 	case SIOCSIFADDR:
+		ifp->if_flags |= IFF_UP;
+		break;
 	case SIOCSIFDSTADDR: 
-		if ((error = suser(p->p_ucred, &p->p_acflag)) != 0)
-			break;
-		/*
-		 * set tunnel endpoints in case that we "only"
-		 * have ip over ip encapsulation. This allows to
-		 * set tunnel endpoints with ifconfig.
-		 */
-		if (ifa->ifa_addr->sa_family == AF_INET) {
-			sa = ifa->ifa_addr;
-			sc->g_src = (satosin(sa))->sin_addr;
-			sc->g_dst = ia->ia_dstaddr.sin_addr;
-			if ((sc->g_src.s_addr != INADDR_ANY) &&
-			    (sc->g_dst.s_addr != INADDR_ANY)) {
-				if (sc->route.ro_rt != 0) /* free old route */
-					RTFREE(sc->route.ro_rt);
-				if (gre_compute_route(sc) == 0)
-					ifp->if_flags |= IFF_UP;
-			}
-		}
 		break;
 	case SIOCSIFFLAGS:
 		if ((error = suser(p->p_ucred, &p->p_acflag)) != 0)
>Release-Note:
>Audit-Trail:

From: Martin Husemann <martin@duskware.de>
To: itojun@itojun.org
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: kern/17202: SIOCSIFADDR/IFDSTADDR on gre* interface destroys outer IP address setting
Date: Mon, 10 Jun 2002 21:52:19 +0200

 >  	case SIOCSIFADDR:
 > +		ifp->if_flags |= IFF_UP;
 > +		break;

 As a side effect (more or less) the old code tried to avoid setting IFF_UP
 if gre_compute_route() had not (yet) succeeded. Infinite recursion and kernel
 stack overflow would happen otherwise.

 This is no longer true with the code you propose now.

 I don't object this change, but we need to properly deal with recursion before
 applying this, IMHO. At least in the (hackish) way if_gif.c does it, or in
 the way you proposed when we discussed this on tech-kern some time ago.

 Martin

From: itojun@iijlab.net
To: Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: kern/17202: SIOCSIFADDR/IFDSTADDR on gre* interface destroys outer IP address setting 
Date: Tue, 11 Jun 2002 05:09:18 +0900

 >>  	case SIOCSIFADDR:
 >> +		ifp->if_flags |= IFF_UP;
 >> +		break;
 >
 >As a side effect (more or less) the old code tried to avoid setting IFF_UP
 >if gre_compute_route() had not (yet) succeeded. Infinite recursion and kernel
 >stack overflow would happen otherwise.
 >
 >This is no longer true with the code you propose now.
 >
 >I don't object this change, but we need to properly deal with recursion before
 >applying this, IMHO. At least in the (hackish) way if_gif.c does it, or in
 >the way you proposed when we discussed this on tech-kern some time ago.

 	could you look at the one I've committed?  it uses IFF_RUNNING for
 	checking if proper route setting was made.  "IFF_RUNNING" is for
 	interface resource allocation, so i think the use makes sense.

 	solutions with m_aux would be nicer, yes.

 itojun
>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.