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:
(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.