CURRENT: ifconfig tap0 results in core dump

[Peter Edwards]

> > % 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.
-------------- next part --------------
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);