Author: mav
Date: Mon Aug 18 15:54:35 2014
New Revision: 270136
URL: http://svnweb.freebsd.org/changeset/base/270136

Log:
  MFC r269492:
  Improve locking of multicast addresses in VLAN and LAGG interfaces.
  
  This fixes several scenarios of reproducible panics, cause by races
  between multicast address changes and interface destruction.

Modified:
  stable/10/sys/net/if_lagg.c
  stable/10/sys/net/if_lagg.h
  stable/10/sys/net/if_vlan.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/net/if_lagg.c
==============================================================================
--- stable/10/sys/net/if_lagg.c Mon Aug 18 14:47:13 2014        (r270135)
+++ stable/10/sys/net/if_lagg.c Mon Aug 18 15:54:35 2014        (r270136)
@@ -1219,39 +1219,39 @@ lagg_ether_cmdmulti(struct lagg_port *lp
        struct ifnet *ifp = lp->lp_ifp;
        struct ifnet *scifp = sc->sc_ifp;
        struct lagg_mc *mc;
-       struct ifmultiaddr *ifma, *rifma = NULL;
-       struct sockaddr_dl sdl;
+       struct ifmultiaddr *ifma;
        int error;
 
        LAGG_WLOCK_ASSERT(sc);
 
-       bzero((char *)&sdl, sizeof(sdl));
-       sdl.sdl_len = sizeof(sdl);
-       sdl.sdl_family = AF_LINK;
-       sdl.sdl_type = IFT_ETHER;
-       sdl.sdl_alen = ETHER_ADDR_LEN;
-       sdl.sdl_index = ifp->if_index;
-
        if (set) {
+               IF_ADDR_WLOCK(scifp);
                TAILQ_FOREACH(ifma, &scifp->if_multiaddrs, ifma_link) {
                        if (ifma->ifma_addr->sa_family != AF_LINK)
                                continue;
-                       bcopy(LLADDR((struct sockaddr_dl *)ifma->ifma_addr),
-                           LLADDR(&sdl), ETHER_ADDR_LEN);
-
-                       error = if_addmulti(ifp, (struct sockaddr *)&sdl, 
&rifma);
-                       if (error)
-                               return (error);
                        mc = malloc(sizeof(struct lagg_mc), M_DEVBUF, M_NOWAIT);
-                       if (mc == NULL)
+                       if (mc == NULL) {
+                               IF_ADDR_WUNLOCK(scifp);
                                return (ENOMEM);
-                       mc->mc_ifma = rifma;
+                       }
+                       bcopy(ifma->ifma_addr, &mc->mc_addr,
+                           ifma->ifma_addr->sa_len);
+                       mc->mc_addr.sdl_index = ifp->if_index;
+                       mc->mc_ifma = NULL;
                        SLIST_INSERT_HEAD(&lp->lp_mc_head, mc, mc_entries);
                }
+               IF_ADDR_WUNLOCK(scifp);
+               SLIST_FOREACH (mc, &lp->lp_mc_head, mc_entries) {
+                       error = if_addmulti(ifp,
+                           (struct sockaddr *)&mc->mc_addr, &mc->mc_ifma);
+                       if (error)
+                               return (error);
+               }
        } else {
                while ((mc = SLIST_FIRST(&lp->lp_mc_head)) != NULL) {
                        SLIST_REMOVE(&lp->lp_mc_head, mc, lagg_mc, mc_entries);
-                       if_delmulti_ifma(mc->mc_ifma);
+                       if (mc->mc_ifma && !lp->lp_detaching)
+                               if_delmulti_ifma(mc->mc_ifma);
                        free(mc, M_DEVBUF);
                }
        }

Modified: stable/10/sys/net/if_lagg.h
==============================================================================
--- stable/10/sys/net/if_lagg.h Mon Aug 18 14:47:13 2014        (r270135)
+++ stable/10/sys/net/if_lagg.h Mon Aug 18 15:54:35 2014        (r270136)
@@ -174,6 +174,7 @@ struct lagg_lb {
 };
 
 struct lagg_mc {
+       struct sockaddr_dl      mc_addr;
        struct ifmultiaddr      *mc_ifma;
        SLIST_ENTRY(lagg_mc)    mc_entries;
 };

Modified: stable/10/sys/net/if_vlan.c
==============================================================================
--- stable/10/sys/net/if_vlan.c Mon Aug 18 14:47:13 2014        (r270135)
+++ stable/10/sys/net/if_vlan.c Mon Aug 18 15:54:35 2014        (r270136)
@@ -458,48 +458,48 @@ trunk_destroy(struct ifvlantrunk *trunk)
  * traffic that it doesn't really want, which ends up being discarded
  * later by the upper protocol layers. Unfortunately, there's no way
  * to avoid this: there really is only one physical interface.
- *
- * XXX: There is a possible race here if more than one thread is
- *      modifying the multicast state of the vlan interface at the same time.
  */
 static int
 vlan_setmulti(struct ifnet *ifp)
 {
        struct ifnet            *ifp_p;
-       struct ifmultiaddr      *ifma, *rifma = NULL;
+       struct ifmultiaddr      *ifma;
        struct ifvlan           *sc;
        struct vlan_mc_entry    *mc;
        int                     error;
 
-       /*VLAN_LOCK_ASSERT();*/
-
        /* Find the parent. */
        sc = ifp->if_softc;
+       TRUNK_LOCK_ASSERT(TRUNK(sc));
        ifp_p = PARENT(sc);
 
        CURVNET_SET_QUIET(ifp_p->if_vnet);
 
        /* First, remove any existing filter entries. */
        while ((mc = SLIST_FIRST(&sc->vlan_mc_listhead)) != NULL) {
-               error = if_delmulti(ifp_p, (struct sockaddr *)&mc->mc_addr);
-               if (error)
-                       return (error);
                SLIST_REMOVE_HEAD(&sc->vlan_mc_listhead, mc_entries);
+               (void)if_delmulti(ifp_p, (struct sockaddr *)&mc->mc_addr);
                free(mc, M_VLAN);
        }
 
        /* Now program new ones. */
+       IF_ADDR_WLOCK(ifp);
        TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
                if (ifma->ifma_addr->sa_family != AF_LINK)
                        continue;
                mc = malloc(sizeof(struct vlan_mc_entry), M_VLAN, M_NOWAIT);
-               if (mc == NULL)
+               if (mc == NULL) {
+                       IF_ADDR_WUNLOCK(ifp);
                        return (ENOMEM);
+               }
                bcopy(ifma->ifma_addr, &mc->mc_addr, ifma->ifma_addr->sa_len);
                mc->mc_addr.sdl_index = ifp_p->if_index;
                SLIST_INSERT_HEAD(&sc->vlan_mc_listhead, mc, mc_entries);
+       }
+       IF_ADDR_WUNLOCK(ifp);
+       SLIST_FOREACH (mc, &sc->vlan_mc_listhead, mc_entries) {
                error = if_addmulti(ifp_p, (struct sockaddr *)&mc->mc_addr,
-                   &rifma);
+                   NULL);
                if (error)
                        return (error);
        }
@@ -1372,7 +1372,7 @@ vlan_unconfig_locked(struct ifnet *ifp, 
                 * Check if we were the last.
                 */
                if (trunk->refcnt == 0) {
-                       trunk->parent->if_vlantrunk = NULL;
+                       parent->if_vlantrunk = NULL;
                        /*
                         * XXXGL: If some ithread has already entered
                         * vlan_input() and is now blocked on the trunk
@@ -1566,6 +1566,7 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
        struct ifreq *ifr;
        struct ifaddr *ifa;
        struct ifvlan *ifv;
+       struct ifvlantrunk *trunk;
        struct vlanreq vlr;
        int error = 0;
 
@@ -1710,8 +1711,12 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
                 * If we don't have a parent, just remember the membership for
                 * when we do.
                 */
-               if (TRUNK(ifv) != NULL)
+               trunk = TRUNK(ifv);
+               if (trunk != NULL) {
+                       TRUNK_LOCK(trunk);
                        error = vlan_setmulti(ifp);
+                       TRUNK_UNLOCK(trunk);
+               }
                break;
 
        default:
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to