svn commit: r238309 - head/sys/net
Author: trociny Date: Mon Jul 9 20:38:18 2012 New Revision: 238309 URL: http://svn.freebsd.org/changeset/base/238309 Log: 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: head/sys/net/if_epair.c Modified: head/sys/net/if_epair.c == --- head/sys/net/if_epair.c Mon Jul 9 20:11:32 2012(r238308) +++ head/sys/net/if_epair.c Mon Jul 9 20:38:18 2012(r238309) @@ -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); ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r238309 - head/sys/net
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 07/09/12 13:38, Mikolaj Golub wrote: Author: trociny Date: Mon Jul 9 20:38:18 2012 New Revision: 238309 URL: http://svn.freebsd.org/changeset/base/238309 Log: 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 Thanks! Since this affects RELENG_9 and RELENG_8, could you please also MFC after a settle period? Cheers, - -- Xin LI delp...@delphij.nethttps://www.delphij.net/ FreeBSD - The Power to Serve! Live free or die -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (FreeBSD) iQEcBAEBCAAGBQJP+0PMAAoJEG80Jeu8UPuz/ugH/2RmmdfCapeP9eQkIPkaImpm D5ghJ0fS6dOM87i5QY6c0rJU2TrcLZHkZGnuYv+BSMqCz5de8dw9s7UMM6sHLL92 i4tgpDE2DQxM4b5skR7yCBRES6IAiY6kDIgVskaS95PxA9wuJA4ohqS8pQ5Tp6h4 lM2urad7+FpYGLZRTWY2yOzgS/g2JZNEGSX6tPIAhg8xX1hryP869zjjHJGd4932 X6pPyRtNdTHB69t00UiVMbJPzcSLtue4ECTms0xCPKC7t0+VZXgiYWOJlNnPQYzy lUkcYy8ZIyUUTD0duOdyun4oQ7xNEdZgZxcRI0Ids68AQs2xUoddL0IyX1ZMe9w= =kSZl -END PGP SIGNATURE- ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r238309 - head/sys/net
On Mon, 09 Jul 2012 13:49:17 -0700 Xin Li wrote: XL On 07/09/12 13:38, Mikolaj Golub wrote: Author: trociny Date: Mon Jul 9 20:38:18 2012 New Revision: 238309 URL: http://svn.freebsd.org/changeset/base/238309 Log: 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 XL Thanks! XL Since this affects RELENG_9 and RELENG_8, could you please also MFC XL after a settle period? Sure. Just forgot to add the 'MFC after' reminder. I am going to MFC it after stable/9 unfreeze unless someone really wants it in 9.1 and tests it a little. This does not look like a critical issue because of the existing workaround (which can be considered as a best practice): move both ends to the home vnet before destroying the epair. -- Mikolaj Golub ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
