From owner-svn-src-all@freebsd.org Thu Sep 21 20:16:12 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2AF0AE1F53B; Thu, 21 Sep 2017 20:16:12 +0000 (UTC) (envelope-from eugen@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0149C687D1; Thu, 21 Sep 2017 20:16:11 +0000 (UTC) (envelope-from eugen@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v8LKGBji024413; Thu, 21 Sep 2017 20:16:11 GMT (envelope-from eugen@FreeBSD.org) Received: (from eugen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v8LKGBMi024412; Thu, 21 Sep 2017 20:16:11 GMT (envelope-from eugen@FreeBSD.org) Message-Id: <201709212016.v8LKGBMi024412@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: eugen set sender to eugen@FreeBSD.org using -f From: Eugene Grosbein Date: Thu, 21 Sep 2017 20:16:11 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r323873 - head/sys/netgraph X-SVN-Group: head X-SVN-Commit-Author: eugen X-SVN-Commit-Paths: head/sys/netgraph X-SVN-Commit-Revision: 323873 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Sep 2017 20:16:12 -0000 Author: eugen (ports committer) Date: Thu Sep 21 20:16:10 2017 New Revision: 323873 URL: https://svnweb.freebsd.org/changeset/base/323873 Log: Unprotected modification of ng_iface(4) private data leads to kernel panic. Fix a race with per-node read-mostly lock and refcounting for a hook. PR: 220076 Tested by: peixoto.cassiano Approved by: avg (mentor), mav (mentor) MFC after: 1 week Relnotes: yes Differential Revision: https://reviews.freebsd.org/D12435 Modified: head/sys/netgraph/ng_iface.c Modified: head/sys/netgraph/ng_iface.c ============================================================================== --- head/sys/netgraph/ng_iface.c Thu Sep 21 20:13:03 2017 (r323872) +++ head/sys/netgraph/ng_iface.c Thu Sep 21 20:16:10 2017 (r323873) @@ -64,6 +64,7 @@ #include #include #include +#include #include #include #include @@ -112,9 +113,15 @@ struct ng_iface_private { int unit; /* Interface unit number */ node_p node; /* Our netgraph node */ hook_p hooks[NUM_FAMILIES]; /* Hook for each address family */ + struct rmlock lock; /* Protect private data changes */ }; typedef struct ng_iface_private *priv_p; +#define PRIV_RLOCK(priv, t) rm_rlock(&priv->lock, t) +#define PRIV_RUNLOCK(priv, t) rm_runlock(&priv->lock, t) +#define PRIV_WLOCK(priv) rm_wlock(&priv->lock) +#define PRIV_WUNLOCK(priv) rm_wunlock(&priv->lock) + /* Interface methods */ static void ng_iface_start(struct ifnet *ifp); static int ng_iface_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data); @@ -431,8 +438,10 @@ ng_iface_bpftap(struct ifnet *ifp, struct mbuf *m, sa_ static int ng_iface_send(struct ifnet *ifp, struct mbuf *m, sa_family_t sa) { + struct rm_priotracker priv_tracker; const priv_p priv = (priv_p) ifp->if_softc; const iffam_p iffam = get_iffam_from_af(sa); + hook_p hook; int error; int len; @@ -446,10 +455,20 @@ ng_iface_send(struct ifnet *ifp, struct mbuf *m, sa_fa /* Copy length before the mbuf gets invalidated. */ len = m->m_pkthdr.len; - /* Send packet. If hook is not connected, mbuf will get freed. */ + PRIV_RLOCK(priv, &priv_tracker); + hook = *get_hook_from_iffam(priv, iffam); + if (hook == NULL) { + NG_FREE_M(m); + PRIV_RUNLOCK(priv, &priv_tracker); + return ENETDOWN; + } + NG_HOOK_REF(hook); + PRIV_RUNLOCK(priv, &priv_tracker); + NG_OUTBOUND_THREAD_REF(); - NG_SEND_DATA_ONLY(error, *get_hook_from_iffam(priv, iffam), m); + NG_SEND_DATA_ONLY(error, hook, m); NG_OUTBOUND_THREAD_UNREF(); + NG_HOOK_UNREF(hook); /* Update stats. */ if (error == 0) { @@ -516,6 +535,8 @@ ng_iface_constructor(node_p node) return (ENOMEM); } + rm_init(&priv->lock, "ng_iface private rmlock"); + /* Link them together */ ifp->if_softc = priv; priv->ifp = ifp; @@ -562,16 +583,21 @@ static int ng_iface_newhook(node_p node, hook_p hook, const char *name) { const iffam_p iffam = get_iffam_from_name(name); + const priv_p priv = NG_NODE_PRIVATE(node); hook_p *hookptr; if (iffam == NULL) return (EPFNOSUPPORT); - hookptr = get_hook_from_iffam(NG_NODE_PRIVATE(node), iffam); - if (*hookptr != NULL) + PRIV_WLOCK(priv); + hookptr = get_hook_from_iffam(priv, iffam); + if (*hookptr != NULL) { + PRIV_WUNLOCK(priv); return (EISCONN); + } *hookptr = hook; NG_HOOK_HI_STACK(hook); NG_HOOK_SET_TO_INBOUND(hook); + PRIV_WUNLOCK(priv); return (0); } @@ -730,6 +756,7 @@ ng_iface_shutdown(node_p node) CURVNET_RESTORE(); priv->ifp = NULL; free_unr(V_ng_iface_unit, priv->unit); + rm_destroy(&priv->lock); free(priv, M_NETGRAPH_IFACE); NG_NODE_SET_PRIVATE(node, NULL); NG_NODE_UNREF(node); @@ -748,7 +775,9 @@ ng_iface_disconnect(hook_p hook) if (iffam == NULL) panic("%s", __func__); + PRIV_WLOCK(priv); *get_hook_from_iffam(priv, iffam) = NULL; + PRIV_WUNLOCK(priv); return (0); }