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 <[EMAIL PROTECTED]>
Date: May 19, 2005 4:34 PM
Subject: Re: CURRENT: ifconfig tap0 results in core dump
To: Matti Saarinen <[EMAIL PROTECTED]>, Scot Hetzel
<[EMAIL PROTECTED]>, freebsd-current@freebsd.org
Cc: [EMAIL PROTECTED]
> > % ifconfig tap0
> > tap0: flags=8802<BROADCAST,SIMPLEX,MULTICAST> 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.
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);
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
