From owner-freebsd-net@FreeBSD.ORG Tue May 24 15:38:03 2005 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7A91D16A41C for ; Tue, 24 May 2005 15:38:03 +0000 (GMT) (envelope-from peadar.edwards@gmail.com) Received: from zproxy.gmail.com (zproxy.gmail.com [64.233.162.201]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1A63543D1D for ; Tue, 24 May 2005 15:38:03 +0000 (GMT) (envelope-from peadar.edwards@gmail.com) Received: by zproxy.gmail.com with SMTP id 34so2002326nzf for ; Tue, 24 May 2005 08:38:02 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:reply-to:to:subject:in-reply-to:mime-version:content-type:references; b=JHhypdIklNG0IbCw8G88fgfXvZIdUZiOmkatdLukd0dbPA5mEjKjcNdwLHKxo0sWk2v3jDs7rzR9eQsXpbY90YgvLFSga1qWuOd9lO9L8RDJyO8ZYmceM+0YpdyN69ZdOB6P+tfq0kNlHaFgj5oVUVGqPltRVzcbSEtVrlEL5E0= Received: by 10.36.10.18 with SMTP id 18mr383441nzj; Tue, 24 May 2005 08:38:02 -0700 (PDT) Received: by 10.36.68.15 with HTTP; Tue, 24 May 2005 08:38:02 -0700 (PDT) Message-ID: <34cb7c8405052408384999ef7a@mail.gmail.com> Date: Tue, 24 May 2005 16:38:02 +0100 From: Peter Edwards To: freebsd-net@freebsd.org In-Reply-To: <34cb7c84050519083477639cd5@mail.gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_21265_26378769.1116949082504" References: <790a9fff0505190809428abb15@mail.gmail.com> <34cb7c84050519083477639cd5@mail.gmail.com> Subject: [patch for review] Fwd: CURRENT: ifconfig tap0 results in core dump X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: peadar@freebsd.org List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 May 2005 15:38:03 -0000 ------=_Part_21265_26378769.1116949082504 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Does anyone have any objection to me committing the patch in this thread? (Note: I inadvertently included a local change that no longer prevents non-root users from opening up /dev/tap*: I don't intend to commit that part of it) ---------- Forwarded message ---------- From: Peter Edwards Date: May 19, 2005 4:34 PM Subject: Re: CURRENT: ifconfig tap0 results in core dump To: Matti Saarinen , Scot Hetzel , freebsd-current@freebsd.org Cc: peadar@freebsd.org > > % ifconfig tap0 > > tap0: flags=3D8802 mtu 1500 > > inet6 fe80::2bd:9ff:fe7c:100%tap0 prefixlen 64 scopeid 0x5 > > zsh: segmentation fault (core dumped) ifconfig tap0 > > > > > > I remember that ifconfig didn't dump core when my laptop ran CURRENT > > from a few months ago. > > > You'll probably need to build a version of ifconfig with debugging > symbols. And then provide a backtrace of the core dump. > > How soon after killing openvpn, do you use the ifconfig command. It > might be possible that devfs was in the process of removing tap0, when > you used the ifconfig command. > Hm. It looks like the "close" code for if_tap clears out the addresses of the interface with a pretty blunt-edged "bzero", rather than removing them in any clean fashion. As a result, ifconfig gets confused over the address families in the tags it sees on the addresses it enumerates off the tap interface, and collapses with a corefile. if_tap's "close" seems to be trying to do part of what's done in if_detach, so I split out what I think are the relevant bits from there and used it in both places. Any networking experts care to take a look at the patch? I suspect there's a whole mess of locking I'm not doing for a start, but I think it might be an improvement over the current situation. Cheers, Peadar. ------=_Part_21265_26378769.1116949082504 Content-Type: text/plain; name=iftap.txt; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="iftap.txt" Index: net/if.c =================================================================== RCS file: /usr/cvs/FreeBSD-CVS/src/sys/net/if.c,v retrieving revision 1.227 diff -u -w -r1.227 if.c --- net/if.c 20 Apr 2005 09:30:54 -0000 1.227 +++ net/if.c 9 May 2005 15:33:40 -0000 @@ -530,13 +530,52 @@ } /* + * Remove any network addresses from an interface. + */ + +void +if_purgeaddrs(struct ifnet *ifp) +{ + struct ifaddr *ifa, *next; + + TAILQ_FOREACH_SAFE(ifa, &ifp->if_addrhead, ifa_link, next) { + + if (ifa->ifa_addr->sa_family == AF_LINK) + continue; +#ifdef INET + /* XXX: Ugly!! ad hoc just for INET */ + if (ifa->ifa_addr && ifa->ifa_addr->sa_family == AF_INET) { + struct ifaliasreq ifr; + + bzero(&ifr, sizeof(ifr)); + ifr.ifra_addr = *ifa->ifa_addr; + if (ifa->ifa_dstaddr) + ifr.ifra_broadaddr = *ifa->ifa_dstaddr; + if (in_control(NULL, SIOCDIFADDR, (caddr_t)&ifr, ifp, + NULL) == 0) + continue; + } +#endif /* INET */ +#ifdef INET6 + if (ifa->ifa_addr && ifa->ifa_addr->sa_family == AF_INET6) { + in6_purgeaddr(ifa); + /* ifp_addrhead is already updated */ + continue; + } +#endif /* INET6 */ + TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link); + IFAFREE(ifa); + } +} + +/* * Detach an interface, removing it from the * list of "active" interfaces. */ void if_detach(struct ifnet *ifp) { - struct ifaddr *ifa, *next; + struct ifaddr *ifa; struct radix_node_head *rnh; int s; int i; @@ -568,35 +607,9 @@ altq_detach(&ifp->if_snd); #endif - for (ifa = TAILQ_FIRST(&ifp->if_addrhead); ifa; ifa = next) { - next = TAILQ_NEXT(ifa, ifa_link); + if_purgeaddrs(ifp); - if (ifa->ifa_addr->sa_family == AF_LINK) - continue; -#ifdef INET - /* XXX: Ugly!! ad hoc just for INET */ - if (ifa->ifa_addr && ifa->ifa_addr->sa_family == AF_INET) { - struct ifaliasreq ifr; - bzero(&ifr, sizeof(ifr)); - ifr.ifra_addr = *ifa->ifa_addr; - if (ifa->ifa_dstaddr) - ifr.ifra_broadaddr = *ifa->ifa_dstaddr; - if (in_control(NULL, SIOCDIFADDR, (caddr_t)&ifr, ifp, - NULL) == 0) - continue; - } -#endif /* INET */ -#ifdef INET6 - if (ifa->ifa_addr && ifa->ifa_addr->sa_family == AF_INET6) { - in6_purgeaddr(ifa); - /* ifp_addrhead is already updated */ - continue; - } -#endif /* INET6 */ - TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link); - IFAFREE(ifa); - } #ifdef INET6 /* Index: net/if_tap.c =================================================================== RCS file: /usr/cvs/FreeBSD-CVS/src/sys/net/if_tap.c,v retrieving revision 1.53 diff -u -w -r1.53 if_tap.c --- net/if_tap.c 4 May 2005 18:55:02 -0000 1.53 +++ net/if_tap.c 9 May 2005 21:01:52 -0000 @@ -356,9 +356,6 @@ struct ifnet *ifp = NULL; int s; - if (tapuopen == 0 && suser(td) != 0) - return (EPERM); - if ((dev2unit(dev) & CLONE_UNITMASK) > TAPMAXUNIT) return (ENXIO); @@ -408,6 +405,7 @@ int bar; struct thread *td; { + struct ifaddr *ifa; struct tap_softc *tp = dev->si_drv1; struct ifnet *ifp = &tp->tap_if; int s; @@ -426,24 +424,10 @@ s = splimp(); if_down(ifp); if (ifp->if_flags & IFF_RUNNING) { - /* find internet addresses and delete routes */ - struct ifaddr *ifa = NULL; - - /* In desparate need of ifaddr locking. */ TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { - if (ifa->ifa_addr->sa_family == AF_INET) { rtinit(ifa, (int)RTM_DELETE, 0); - - /* remove address from interface */ - bzero(ifa->ifa_addr, - sizeof(*(ifa->ifa_addr))); - bzero(ifa->ifa_dstaddr, - sizeof(*(ifa->ifa_dstaddr))); - bzero(ifa->ifa_netmask, - sizeof(*(ifa->ifa_netmask))); } - } - + if_purgeaddrs(ifp); ifp->if_flags &= ~IFF_RUNNING; } splx(s); Index: net/if_var.h =================================================================== RCS file: /usr/cvs/FreeBSD-CVS/src/sys/net/if_var.h,v retrieving revision 1.95 diff -u -w -r1.95 if_var.h --- net/if_var.h 20 Apr 2005 09:30:54 -0000 1.95 +++ net/if_var.h 9 May 2005 15:33:41 -0000 @@ -629,6 +629,7 @@ void if_attach(struct ifnet *); int if_delmulti(struct ifnet *, struct sockaddr *); void if_detach(struct ifnet *); +void if_purgeaddrs(struct ifnet *); void if_down(struct ifnet *); void if_initname(struct ifnet *, const char *, int); void if_link_state_change(struct ifnet *, int); ------=_Part_21265_26378769.1116949082504--