Date: Wed, 23 Dec 2020 05:12:33 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: cd698c51790e - netgraph: Fix ng_ether's shutdown handing Message-ID: <202012230512.0BN5CXUq047776@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=cd698c51790e956fed0975f451d3dfc361dc7c24 commit cd698c51790e956fed0975f451d3dfc361dc7c24 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2020-12-23 05:11:16 +0000 Commit: Mark Johnston <markj@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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202012230512.0BN5CXUq047776>