From 48c19ec8c39dce9d43382c3506b5d95443a5f891 Mon Sep 17 00:00:00 2001 From: mav Date: Mon, 18 Aug 2014 15:54:35 +0000 Subject: [PATCH] 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. git-svn-id: svn://svn.freebsd.org/base/stable/10@270136 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/net/if_lagg.c | 36 ++++++++++++++++++------------------ sys/net/if_lagg.h | 1 + sys/net/if_vlan.c | 31 ++++++++++++++++++------------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/sys/net/if_lagg.c b/sys/net/if_lagg.c index bf4d9d78f..4d41e636d 100644 --- a/sys/net/if_lagg.c +++ b/sys/net/if_lagg.c @@ -1219,39 +1219,39 @@ lagg_ether_cmdmulti(struct lagg_port *lp, int set) 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); } } diff --git a/sys/net/if_lagg.h b/sys/net/if_lagg.h index e52f9014c..4a1d9a0a7 100644 --- a/sys/net/if_lagg.h +++ b/sys/net/if_lagg.h @@ -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; }; diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c index e9216db88..9d547a652 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -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, int departing) * 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, caddr_t data) 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, caddr_t data) * 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: -- 2.45.0