git: cd698c51790e - netgraph: Fix ng_ether's shutdown handing

Mark Johnston markj at FreeBSD.org
Wed Dec 23 05:12:34 UTC 2020


The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=cd698c51790e956fed0975f451d3dfc361dc7c24

commit cd698c51790e956fed0975f451d3dfc361dc7c24
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2020-12-23 05:11:16 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2020-12-23 05:12:16 +0000

    netgraph: Fix ng_ether's shutdown handing
    
    When tearing down a VNET, netgraph sends shutdown messages to all of the
    nodes before detaching interfaces (SI_SUB_NETGRAPH comes before
    SI_SUB_INIT_IF in teardown order).  ng_ether nodes handle this by
    destroying themselves without detaching from the parent ifnet.  Then,
    when ifnets go away they detach their ng_ether nodes again, triggering a
    use-after-free.
    
    Handle this by modifying ng_ether_shutdown() to detach from the ifnet.
    If the shutdown was triggered by an ifnet being destroyed, we will clear
    priv->ifp in the ng_ether detach callback, so priv->ifp may be NULL.
    
    Also get rid of the printf in vnet_netgraph_uninit().  It can be
    triggered trivially by ng_ether since ng_ether_shutdown() persists the
    node unless NG_REALLY_DIE is set.
    
    PR:             233622
    Reviewed by:    afedorov, kp, Lutz Donnerhacke
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D27662
---
 sys/netgraph/ng_base.c  |  4 +---
 sys/netgraph/ng_ether.c | 13 ++++++-------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/sys/netgraph/ng_base.c b/sys/netgraph/ng_base.c
index c5b38dc1fdd6..dadf86eb8dde 100644
--- a/sys/netgraph/ng_base.c
+++ b/sys/netgraph/ng_base.c
@@ -3166,12 +3166,10 @@ vnet_netgraph_uninit(const void *unused __unused)
 		/* Attempt to kill it only if it is a regular node */
 		if (node != NULL) {
 			if (node == last_killed) {
-				/* This should never happen */
-				printf("ng node %s needs NGF_REALLY_DIE\n",
-				    node->nd_name);
 				if (node->nd_flags & NGF_REALLY_DIE)
 					panic("ng node %s won't die",
 					    node->nd_name);
+				/* The node persisted itself.  Try again. */
 				node->nd_flags |= NGF_REALLY_DIE;
 			}
 			ng_rmnode(node, NULL, NULL, 0);
diff --git a/sys/netgraph/ng_ether.c b/sys/netgraph/ng_ether.c
index 0d0d5f990f84..5718de235c4c 100644
--- a/sys/netgraph/ng_ether.c
+++ b/sys/netgraph/ng_ether.c
@@ -414,8 +414,7 @@ ng_ether_ifnet_arrival_event(void *arg __unused, struct ifnet *ifp)
 	node_p node;
 
 	/* Only ethernet interfaces are of interest. */
-	if (ifp->if_type != IFT_ETHER
-	    && ifp->if_type != IFT_L2VLAN)
+	if (ifp->if_type != IFT_ETHER && ifp->if_type != IFT_L2VLAN)
 		return;
 
 	/*
@@ -753,13 +752,13 @@ ng_ether_shutdown(node_p node)
 
 	if (node->nd_flags & NGF_REALLY_DIE) {
 		/*
-		 * WE came here because the ethernet card is being unloaded,
-		 * so stop being persistent.
-		 * Actually undo all the things we did on creation.
-		 * Assume the ifp has already been freed.
+		 * The ifnet is going away, perhaps because the driver was
+		 * unloaded or its vnet is being torn down.
 		 */
 		NG_NODE_SET_PRIVATE(node, NULL);
-		free(priv, M_NETGRAPH);		
+		if (priv->ifp != NULL)
+			IFP2NG(priv->ifp) = NULL;
+		free(priv, M_NETGRAPH);
 		NG_NODE_UNREF(node);	/* free node itself */
 		return (0);
 	}


More information about the dev-commits-src-all mailing list