svn commit: r353967 - head/sys/net

[Eric Joyner]

Author: erj
Date: Wed Oct 23 23:20:49 2019
New Revision: 353967
URL: https://svnweb.freebsd.org/changeset/base/353967

Log:
  iflib: call ether_ifdetach and netmap_detach before stop
  
  From Jake:
  Calling ether_ifdetach after iflib_stop leads to a potential race where
  a stale ifp pointer can remain in the route entry list for IPv6 traffic.
  This will potentially cause a page fault or other system instability if
  the ifp pointer is accessed.
  
  Move both iflib_netmap_detach and ether_ifdetach to be called prior to
  iflib_stop. This avoids the race above, and helps ensure that other ifp
  references are removed before stopping the interface.
  
  Submitted by:	Jacob Keller <jacob.e.keller at intel.com>
  Reviewed by:	erj@, gallatin@, jhb@
  MFC after:	3 days
  Sponsored by:	Intel Corporation
  Differential Revision:	https://reviews.freebsd.org/D22071

Modified:
  head/sys/net/iflib.c

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c	Wed Oct 23 23:10:12 2019	(r353966)
+++ head/sys/net/iflib.c	Wed Oct 23 23:20:49 2019	(r353967)
@@ -703,6 +703,7 @@ static int iflib_altq_if_transmit(if_t ifp, struct mbu
 #endif
 static int iflib_register(if_ctx_t);
 static void iflib_deregister(if_ctx_t);
+static void iflib_unregister_vlan_handlers(if_ctx_t ctx);
 static void iflib_init_locked(if_ctx_t ctx);
 static void iflib_add_device_sysctl_pre(if_ctx_t ctx);
 static void iflib_add_device_sysctl_post(if_ctx_t ctx);
@@ -5007,6 +5008,9 @@ iflib_pseudo_deregister(if_ctx_t ctx)
 	struct taskqgroup *tqg;
 	iflib_fl_t fl;
 
+	/* Unregister VLAN event handlers early */
+	iflib_unregister_vlan_handlers(ctx);
+
 	ether_ifdetach(ifp);
 	/* XXX drain any dependent tasks */
 	tqg = qgroup_if_io_tqg;
@@ -5080,12 +5084,16 @@ iflib_device_deregister(if_ctx_t ctx)
 	ctx->ifc_flags |= IFC_IN_DETACH;
 	STATE_UNLOCK(ctx);
 
+	/* Unregister VLAN handlers before calling iflib_stop() */
+	iflib_unregister_vlan_handlers(ctx);
+
+	iflib_netmap_detach(ifp);
+	ether_ifdetach(ifp);
+
 	CTX_LOCK(ctx);
 	iflib_stop(ctx);
 	CTX_UNLOCK(ctx);
 
-	iflib_netmap_detach(ifp);
-	ether_ifdetach(ifp);
 	iflib_rem_pfil(ctx);
 	if (ctx->ifc_led_dev != NULL)
 		led_destroy(ctx->ifc_led_dev);
@@ -5375,13 +5383,8 @@ iflib_register(if_ctx_t ctx)
 }
 
 static void
-iflib_deregister(if_ctx_t ctx)
+iflib_unregister_vlan_handlers(if_ctx_t ctx)
 {
-	if_t ifp = ctx->ifc_ifp;
-
-	/* Remove all media */
-	ifmedia_removeall(&ctx->ifc_media);
-
 	/* Unregister VLAN events */
 	if (ctx->ifc_vlan_attach_event != NULL) {
 		EVENTHANDLER_DEREGISTER(vlan_config, ctx->ifc_vlan_attach_event);
@@ -5391,6 +5394,19 @@ iflib_deregister(if_ctx_t ctx)
 		EVENTHANDLER_DEREGISTER(vlan_unconfig, ctx->ifc_vlan_detach_event);
 		ctx->ifc_vlan_detach_event = NULL;
 	}
+
+}
+
+static void
+iflib_deregister(if_ctx_t ctx)
+{
+	if_t ifp = ctx->ifc_ifp;
+
+	/* Remove all media */
+	ifmedia_removeall(&ctx->ifc_media);
+
+	/* Ensure that VLAN event handlers are unregistered */
+	iflib_unregister_vlan_handlers(ctx);
 
 	/* Release kobject reference */
 	kobj_delete((kobj_t) ctx, NULL);