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);
 }
 
_______________________________________________
svn-src-stable-10@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-stable-10
To unsubscribe, send any mail to "svn-src-stable-10-unsubscr...@freebsd.org"

Reply via email to