Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 1 Sep 2012 10:28:20 +0000 (UTC)
From:      Mikolaj Golub <trociny@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r239981 - stable/8/sys/net
Message-ID:  <201209011028.q81ASKIM093537@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: trociny
Date: Sat Sep  1 10:28:20 2012
New Revision: 239981
URL: http://svn.freebsd.org/changeset/base/239981

Log:
  MFC r238309:
  
  In epair_clone_destroy(), when destroying the second half, we have to
  switch to its vnet before calling ether_ifdetach(). Otherwise if the
  second half resides in a different vnet, if_detach() silently fails
  leaving a stale pointer in V_ifnet list, and the system crashes trying
  to access this pointer later.
  
  Another solution could be not to allow to destroy epair unless both
  ends are in the home vnet.
  
  Discussed with:	bz
  Tested by:	delphij

Modified:
  stable/8/sys/net/if_epair.c
Directory Properties:
  stable/8/sys/   (props changed)

Modified: stable/8/sys/net/if_epair.c
==============================================================================
--- stable/8/sys/net/if_epair.c	Sat Sep  1 10:27:18 2012	(r239980)
+++ stable/8/sys/net/if_epair.c	Sat Sep  1 10:28:20 2012	(r239981)
@@ -904,39 +904,41 @@ epair_clone_destroy(struct if_clone *ifc
 	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 may 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);
+	ifmedia_removeall(&scb->media);
+	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);
 	ifmedia_removeall(&sca->media);
-	ifmedia_removeall(&scb->media);
-	free(scb, M_EPAIR);
 	free(sca, M_EPAIR);
 	ifc_free_unit(ifc, unit);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201209011028.q81ASKIM093537>