d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Unprotected modification of ng_iface(4) private data leads to kernel panic.

Browse Source 
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
This commit is contained in:
Eugene Grosbein 2017-09-21 20:16:10 +00:00
parent b3eaa68045
commit 10633c7e5a
1 changed files with 33 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
sys/netgraph/ng_iface.c
View File

@ -64,6 +64,7 @@
#include <sys/errno.h> #include <sys/errno.h>
#include <sys/proc.h> #include <sys/proc.h>
#include <sys/random.h> #include <sys/random.h>
#include <sys/rmlock.h>
#include <sys/sockio.h> #include <sys/sockio.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/syslog.h> #include <sys/syslog.h>
@ -112,9 +113,15 @@ struct ng_iface_private {
int unit; /* Interface unit number */ int unit; /* Interface unit number */
node_p node; /* Our netgraph node */ node_p node; /* Our netgraph node */
hook_p hooks[NUM_FAMILIES]; /* Hook for each address family */ hook_p hooks[NUM_FAMILIES]; /* Hook for each address family */
struct rmlock lock; /* Protect private data changes */
}; };
typedef struct ng_iface_private *priv_p; 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 */ /* Interface methods */
static void ng_iface_start(struct ifnet *ifp); static void ng_iface_start(struct ifnet *ifp);
static int ng_iface_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data); 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_family_t family)
static int static int
ng_iface_send(struct ifnet *ifp, struct mbuf *m, sa_family_t sa) 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 priv_p priv = (priv_p) ifp->if_softc;
const iffam_p iffam = get_iffam_from_af(sa); const iffam_p iffam = get_iffam_from_af(sa);
hook_p hook;
int error; int error;
int len; int len;
@ -446,10 +455,20 @@ ng_iface_send(struct ifnet *ifp, struct mbuf *m, sa_family_t sa)
/* Copy length before the mbuf gets invalidated. */ /* Copy length before the mbuf gets invalidated. */
len = m->m_pkthdr.len; 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_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_OUTBOUND_THREAD_UNREF();
NG_HOOK_UNREF(hook);
/* Update stats. */ /* Update stats. */
if (error == 0) { if (error == 0) {
@ -516,6 +535,8 @@ ng_iface_constructor(node_p node)
return (ENOMEM); return (ENOMEM);
} }
rm_init(&priv->lock, "ng_iface private rmlock");
/* Link them together */ /* Link them together */
ifp->if_softc = priv; ifp->if_softc = priv;
priv->ifp = ifp; priv->ifp = ifp;
@ -562,16 +583,21 @@ static int
ng_iface_newhook(node_p node, hook_p hook, const char *name) ng_iface_newhook(node_p node, hook_p hook, const char *name)
{ {
const iffam_p iffam = get_iffam_from_name(name); const iffam_p iffam = get_iffam_from_name(name);
const priv_p priv = NG_NODE_PRIVATE(node);
hook_p *hookptr; hook_p *hookptr;
if (iffam == NULL) if (iffam == NULL)
return (EPFNOSUPPORT); return (EPFNOSUPPORT);
hookptr = get_hook_from_iffam(NG_NODE_PRIVATE(node), iffam); PRIV_WLOCK(priv);
if (*hookptr != NULL) hookptr = get_hook_from_iffam(priv, iffam);
if (*hookptr != NULL) {
PRIV_WUNLOCK(priv);
return (EISCONN); return (EISCONN);
}
*hookptr = hook; *hookptr = hook;
NG_HOOK_HI_STACK(hook); NG_HOOK_HI_STACK(hook);
NG_HOOK_SET_TO_INBOUND(hook); NG_HOOK_SET_TO_INBOUND(hook);
PRIV_WUNLOCK(priv);
return (0); return (0);
} }
@ -730,6 +756,7 @@ ng_iface_shutdown(node_p node)
CURVNET_RESTORE(); CURVNET_RESTORE();
priv->ifp = NULL; priv->ifp = NULL;
free_unr(V_ng_iface_unit, priv->unit); free_unr(V_ng_iface_unit, priv->unit);
rm_destroy(&priv->lock);
free(priv, M_NETGRAPH_IFACE); free(priv, M_NETGRAPH_IFACE);
NG_NODE_SET_PRIVATE(node, NULL); NG_NODE_SET_PRIVATE(node, NULL);
NG_NODE_UNREF(node); NG_NODE_UNREF(node);
@ -748,7 +775,9 @@ ng_iface_disconnect(hook_p hook)
if (iffam == NULL) if (iffam == NULL)
panic("%s", __func__); panic("%s", __func__);
PRIV_WLOCK(priv);
*get_hook_from_iffam(priv, iffam) = NULL; *get_hook_from_iffam(priv, iffam) = NULL;
PRIV_WUNLOCK(priv);
return (0); return (0);
} }