Hi,

On CURRENT when doing something like below:

jail -c name=test vnet persist
ifconfig epair0 create
ifconfig epair0b vnet test
ifconfig epair0a destroy
jexec test ifconfig

The system panics on the last command:

#8  0xc0c32f7c in calltrap () at /usr/src/sys/i386/i386/exception.s:168
#9  0xc0994618 in strlen (str=0xdeadc0de <Address 0xdeadc0de out of bounds>)
    at /usr/src/sys/libkern/strlen.c:41
#10 0xc09216c7 in kvprintf (fmt=0xc0d32ec7 " @ %s:%d", 
    func=0xc0920950 <snprintf_func>, arg=0xeb5dfa30, radix=10, 
    ap=0xeb5dfa74 "\202l<D4><C0><C4>\005") at /usr/src/sys/kern/subr_prf.c:728
#11 0xc0921f4b in vsnprintf (str=0xc0ebd700 "mtx_lock() of spin mutex ", 
    size=256, format=0xc0d32eac "mtx_lock() of spin mutex %s @ %s:%d", 
    ap=0xeb5dfa70 "<DE><C0><AD><DE>\202l<D4><C0><C4>\005") at 
/usr/src/sys/kern/subr_prf.c:461
#12 0xc08e914b in panic (fmt=0xc0d32eac "mtx_lock() of spin mutex %s @ %s:%d")
    at /usr/src/sys/kern/kern_shutdown.c:557
#13 0xc08d882a in _mtx_lock_flags (m=0xc2ec7628, opts=0, 
    file=0xc0d46c82 "/usr/src/sys/net/rtsock.c", line=1476)
    at /usr/src/sys/kern/kern_mutex.c:197
#14 0xc09b8389 in sysctl_rtsock (oidp=0xc0e412a0, arg1=0xeb5dfbe4, arg2=4, 
    req=0xeb5dfb70) at /usr/src/sys/net/rtsock.c:1476
#15 0xc08f3eed in sysctl_root (oidp=Variable "oidp" is not available.
) at /usr/src/sys/kern/kern_sysctl.c:1456
#16 0xc08f4145 in userland_sysctl (td=0xc3ab9870, name=0xeb5dfbdc, namelen=6, 
    old=0x0, oldlenp=0xbfbfe538, inkernel=0, new=0x0, newlen=0, 
    retval=0xeb5dfc3c, flags=0) at /usr/src/sys/kern/kern_sysctl.c:1566
#17 0xc08f4514 in __sysctl (td=0xc3ab9870, uap=0xeb5dfcec)
    at /usr/src/sys/kern/kern_sysctl.c:1492
#18 0xc092b793 in syscallenter (td=0xc3ab9870, sa=0xeb5dfce4)
    at /usr/src/sys/kern/subr_trap.c:318
#19 0xc0c4975f in syscall (frame=0xeb5dfd28)
    at /usr/src/sys/i386/i386/trap.c:1094
#20 0xc0c33011 in Xint0x80_syscall ()
    at /usr/src/sys/i386/i386/exception.s:266
#21 0x00000033 in ?? ()
Previous frame inner to this frame (corrupt stack?)

It crashes in sysctl_iflist when traversing V_ifnet list because it contains
dead ifp.

This is because when epair_clone_destroy() calls ether_ifdetach() for its
second half it does not switch to its vnet and if_detach_internal() can't find
the interface and just returns. This can be checked when adding the first
patch from the attaches:

<5>epair20a: link state changed to DOWN
<5>epair20b: link state changed to DOWN
panic: if_detach_internal: ifp=0xc35b8800 not on the ifnet tailq 0xc3e43038

#11 0xc08e91f4 in panic (
    fmt=0xc0d44def "%s: ifp=%p not on the ifnet tailq %p")
    at /usr/src/sys/kern/kern_shutdown.c:574
#12 0xc09a19b1 in if_detach_internal (ifp=0xc35b8800, vmove=0)
    at /usr/src/sys/net/if.c:831
#13 0xc09a4190 in if_detach (ifp=0xc35b8800) at /usr/src/sys/net/if.c:805
#14 0xc09a5fdd in ether_ifdetach (ifp=0xc35b8800)
    at /usr/src/sys/net/if_ethersubr.c:989
#15 0xc3e63ec8 in epair_clone_destroy (ifc=0xc3e661c0, ifp=0xc2ec7400)
    at /usr/src/sys/modules/if_epair/../../net/if_epair.c:871
#16 0xc09a4b17 in if_clone_destroyif (ifc=0xc3e661c0, ifp=0xc2ec7400)
    at /usr/src/sys/net/if_clone.c:269
#17 0xc09a53c7 in if_clone_destroy (name=0xc303e940 "epair20a")
    at /usr/src/sys/net/if_clone.c:230
#18 0xc09a2a21 in ifioctl (so=0xc3aa3680, cmd=2149607801, 
    data=0xc303e940 "epair20a", td=0xc3bcb5a0) at /usr/src/sys/net/if.c:2464
#19 0xc093c977 in soo_ioctl (fp=0xc3aa5000, cmd=2149607801, data=0xc303e940, 
    active_cred=0xc2d8dc80, td=0xc3bcb5a0)
    at /usr/src/sys/kern/sys_socket.c:212
#20 0xc0936aed in kern_ioctl (td=0xc3bcb5a0, fd=3, com=2149607801, 
    data=0xc303e940 "epair20a") at file.h:254
#21 0xc0936c74 in ioctl (td=0xc3bcb5a0, uap=0xeb5fdcec)
    at /usr/src/sys/kern/sys_generic.c:679
#22 0xc092b793 in syscallenter (td=0xc3bcb5a0, sa=0xeb5fdce4)
    at /usr/src/sys/kern/subr_trap.c:318
#23 0xc0c4974f in syscall (frame=0xeb5fdd28)
    at /usr/src/sys/i386/i386/trap.c:1094
#24 0xc0c33001 in Xint0x80_syscall ()
    at /usr/src/sys/i386/i386/exception.s:266

Adding vnet switch to epair_clone_destroy() like below fixes the issue:

-       ether_ifdetach(oifp);
+       {
+               CURVNET_SET_QUIET(oifp->if_vnet);
+               ether_ifdetach(oifp);
+               CURVNET_RESTORE();
+       }
        ether_ifdetach(ifp);

Although it looks a bit ugly :-). What do you think about some reordering like
in the second patch attached?

Also it looks wrong that if_detach_internal() silently returned in that case.
I think it should panic or at least warns (like in the third patch).

-- 
Mikolaj Golub

Index: sys/net/if.c
===================================================================
--- sys/net/if.c	(revision 217617)
+++ sys/net/if.c	(working copy)
@@ -828,11 +828,8 @@ if_detach_internal(struct ifnet *ifp, int vmove)
 #endif
 	IFNET_WUNLOCK();
 	if (!found) {
-		if (vmove)
-			panic("%s: ifp=%p not on the ifnet tailq %p",
-			    __func__, ifp, &V_ifnet);
-		else
-			return; /* XXX this should panic as well? */
+		panic("%s: ifp=%p not on the ifnet tailq %p",
+		    __func__, ifp, &V_ifnet);
 	}
 
 	/*
Index: sys/net/if_epair.c
===================================================================
--- sys/net/if_epair.c	(revision 217596)
+++ sys/net/if_epair.c	(working copy)
@@ -865,38 +865,41 @@ epair_clone_destroy(struct if_clone *ifc, struct i
 	if_link_state_change(oifp, LINK_STATE_DOWN);
 	ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
 	oifp->if_drv_flags &= ~IFF_DRV_RUNNING;
+
+	/*
+	 * Get rid of our second half. As the other of the two
+	 * interfaces my reside in a different vnet, we need to
+	 * switch before freeing them.
+	 */
+	CURVNET_SET_QUIET(oifp->if_vnet);
 	ether_ifdetach(oifp);
-	ether_ifdetach(ifp);
 	/*
 	 * Wait for all packets to be dispatched to if_input.
-	 * The numbers can only go down as the interfaces are
+	 * The numbers can only go down as the interface is
 	 * detached so there is no need to use atomics.
 	 */
-	DPRINTF("sca refcnt=%u scb refcnt=%u\n", sca->refcount, scb->refcount);
-	EPAIR_REFCOUNT_ASSERT(sca->refcount == 1 && scb->refcount == 1,
-	    ("%s: ifp=%p sca->refcount!=1: %d || ifp=%p scb->refcount!=1: %d",
-	    __func__, ifp, sca->refcount, oifp, scb->refcount));
-
-	/*
-	 * Get rid of our second half.
-	 */
+	DPRINTF("scb refcnt=%u\n", scb->refcount);
+	EPAIR_REFCOUNT_ASSERT(scb->refcount == 1,
+	    ("%s: ifp=%p scb->refcount!=1: %d", __func__, oifp, scb->refcount));
 	oifp->if_softc = NULL;
 	error = if_clone_destroyif(ifc, oifp);
 	if (error)
 		panic("%s: if_clone_destroyif() for our 2nd iface failed: %d",
 		    __func__, error);
+	if_free(oifp);
+	free(scb, M_EPAIR);
+	CURVNET_RESTORE();
 
+	ether_ifdetach(ifp);
 	/*
-	 * Finish cleaning up. Free them and release the unit.
-	 * As the other of the two interfaces my reside in a different vnet,
-	 * we need to switch before freeing them.
+	 * Wait for all packets to be dispatched to if_input.
 	 */
-	CURVNET_SET_QUIET(oifp->if_vnet);
-	if_free(oifp);
-	CURVNET_RESTORE();
+	DPRINTF("sca refcnt=%u\n", sca->refcount);
+	EPAIR_REFCOUNT_ASSERT(sca->refcount == 1,
+	    ("%s: ifp=%p sca->refcount!=1: %d", __func__, ifp, sca->refcount));
 	if_free(ifp);
-	free(scb, M_EPAIR);
 	free(sca, M_EPAIR);
+
 	ifc_free_unit(ifc, unit);
 
 	return (0);
Index: sys/net/if.c
===================================================================
--- sys/net/if.c	(revision 217577)
+++ sys/net/if.c	(working copy)
@@ -831,8 +831,11 @@ if_detach_internal(struct ifnet *ifp, int vmove)
 		if (vmove)
 			panic("%s: ifp=%p not on the ifnet tailq %p",
 			    __func__, ifp, &V_ifnet);
-		else
+		else {
+			log(LOG_WARNING, "%s: ifp=%p not on the ifnet tailq %p",
+			    __func__, ifp, &V_ifnet);
 			return; /* XXX this should panic as well? */
+		}
 	}
 
 	/*
_______________________________________________
freebsd-virtualization@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

Reply via email to