Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 1 Oct 2017 19:40:29 +0000 (UTC)
From:      Eugene Grosbein <eugen@FreeBSD.org>
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
Message-ID:  <201710011940.v91JeTOT008117@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
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 <sys/systm.h>
 #include <sys/errno.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/mbuf.h>
 #include <sys/errno.h>
 #include <sys/proc.h>
 #include <sys/random.h>
+#include <sys/rmlock.h>
 #include <sys/sockio.h>
 #include <sys/socket.h>
 #include <sys/syslog.h>
@@ -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);
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201710011940.v91JeTOT008117>