diff options
author | mav <mav@FreeBSD.org> | 2014-08-04 00:58:12 +0000 |
---|---|---|
committer | mav <mav@FreeBSD.org> | 2014-08-04 00:58:12 +0000 |
commit | 1d4e2a0972e43deede50fb73833cea757641264d (patch) | |
tree | 73513a504d7916fb55586906b3983941fb699d27 | |
parent | f53e346683f0398ed0ec1a3a014c5e798c3f92d9 (diff) | |
download | FreeBSD-src-1d4e2a0972e43deede50fb73833cea757641264d.zip FreeBSD-src-1d4e2a0972e43deede50fb73833cea757641264d.tar.gz |
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.
MFC after: 2 weeks
-rw-r--r-- | sys/net/if_lagg.c | 32 | ||||
-rw-r--r-- | sys/net/if_lagg.h | 1 | ||||
-rw-r--r-- | sys/net/if_vlan.c | 31 |
3 files changed, 37 insertions, 27 deletions
diff --git a/sys/net/if_lagg.c b/sys/net/if_lagg.c index ec868e1..cc92acf 100644 --- a/sys/net/if_lagg.c +++ b/sys/net/if_lagg.c @@ -1217,35 +1217,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); - link_init_sdl(ifp, (struct sockaddr *)&sdl, IFT_ETHER); - sdl.sdl_alen = ETHER_ADDR_LEN; - 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 e52f901..4a1d9a0 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 00d99ed..763020b 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -460,48 +460,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); } @@ -1374,7 +1374,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 @@ -1568,6 +1568,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; @@ -1712,8 +1713,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: |