From owner-svn-src-stable-10@freebsd.org Sun Oct 1 19:40:30 2017 Return-Path: Delivered-To: svn-src-stable-10@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 688BBE2CCF8; Sun, 1 Oct 2017 19:40:30 +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 43D5E6C1C8; Sun, 1 Oct 2017 19:40:30 +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 v91JeTBF008118; Sun, 1 Oct 2017 19:40:29 GMT (envelope-from eugen@FreeBSD.org) Received: (from eugen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v91JeTOT008117; Sun, 1 Oct 2017 19:40:29 GMT (envelope-from eugen@FreeBSD.org) Message-Id: <201710011940.v91JeTOT008117@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: eugen set sender to eugen@FreeBSD.org using -f From: Eugene Grosbein Date: Sun, 1 Oct 2017 19:40:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r324176 - stable/10/sys/netgraph X-SVN-Group: stable-10 X-SVN-Commit-Author: eugen X-SVN-Commit-Paths: stable/10/sys/netgraph X-SVN-Commit-Revision: 324176 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-10@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 10-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Oct 2017 19:40:30 -0000 Author: eugen (ports committer) Date: Sun Oct 1 19:40:29 2017 New Revision: 324176 URL: https://svnweb.freebsd.org/changeset/base/324176 Log: MFC r323873, r324081: 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: mav (mentor) Relnotes: yes Differential Revision: https://reviews.freebsd.org/D12435 Modified: stable/10/sys/netgraph/ng_iface.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/netgraph/ng_iface.c ============================================================================== --- stable/10/sys/netgraph/ng_iface.c Sun Oct 1 19:39:27 2017 (r324175) +++ stable/10/sys/netgraph/ng_iface.c Sun Oct 1 19:40:29 2017 (r324176) @@ -61,11 +61,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -116,9 +118,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); @@ -453,8 +461,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; @@ -468,10 +478,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) { @@ -538,6 +558,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; @@ -584,16 +606,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); } @@ -800,6 +827,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); @@ -818,7 +846,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); }