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
This commit is contained in:
Mark Johnston 2020-12-23 00:11:16 -05:00
parent 1cc908de3a
commit cd698c5179
2 changed files with 7 additions and 10 deletions

View File

@ -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);

View File

@ -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);
}