summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoreugen <eugen@FreeBSD.org>2017-10-01 19:40:29 +0000
committereugen <eugen@FreeBSD.org>2017-10-01 19:40:29 +0000
commite1f98c1e457ff061f5691ba012a5d223c2171f2e (patch)
treeefbfd97db30787f00271436700c793ff05f23924
parenta1eb3bb530588a148c1fbf8d20286dc55b306ca1 (diff)
downloadFreeBSD-src-e1f98c1e457ff061f5691ba012a5d223c2171f2e.zip
FreeBSD-src-e1f98c1e457ff061f5691ba012a5d223c2171f2e.tar.gz
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
-rw-r--r--sys/netgraph/ng_iface.c38
1 files changed, 34 insertions, 4 deletions
diff --git a/sys/netgraph/ng_iface.c b/sys/netgraph/ng_iface.c
index 6c18d2a..dc5b133 100644
--- a/sys/netgraph/ng_iface.c
+++ b/sys/netgraph/ng_iface.c
@@ -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_family_t family)
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_family_t sa)
/* 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);
}
OpenPOWER on IntegriCloud