From owner-dev-commits-src-branches@freebsd.org Wed Jan 6 14:57:53 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 359554D32AF; Wed, 6 Jan 2021 14:57:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4D9st910Qfz3vKX; Wed, 6 Jan 2021 14:57:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 152441FFFD; Wed, 6 Jan 2021 14:57:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 106EvrBi047786; Wed, 6 Jan 2021 14:57:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 106EvrJN047785; Wed, 6 Jan 2021 14:57:53 GMT (envelope-from git) Date: Wed, 6 Jan 2021 14:57:53 GMT Message-Id: <202101061457.106EvrJN047785@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 62489e19ebe2 - stable/12 - netgraph: Fix ng_ether's shutdown handing MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 62489e19ebe287034833d07aeb74ca0f0599be6e Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jan 2021 14:57:53 -0000 The branch stable/12 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=62489e19ebe287034833d07aeb74ca0f0599be6e commit 62489e19ebe287034833d07aeb74ca0f0599be6e Author: Mark Johnston AuthorDate: 2020-12-23 05:11:16 +0000 Commit: Mark Johnston CommitDate: 2021-01-06 14:41:53 +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 (cherry picked from commit cd698c51790e956fed0975f451d3dfc361dc7c24) --- 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 a3ff2532ecbd..cc8df1e45aa6 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 575f3740a6f1..770cd2bbbafd 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); }