Author: jhb
Date: Fri Jan 13 19:51:15 2012
New Revision: 230077
URL: http://svn.freebsd.org/changeset/base/230077
Log:
MFC 229390,229420,229479:
Fix some races in the multicast code by removing places where we would
drop the IF_ADDR_LOCK while walking an interface's multicast address list:
- Use TAILQ_FOREACH() instead of TAILQ_FOREACH_SAFE() for some loops that
do not modify the queues they iterate over.
- When cancelling multicast timers on an interface, don't release the
reference on a group in the leaving state while iterating over the loop.
Instead, use the same approach used in igmp_ifdetach() and mld_ifdetach()
of placing the groups to free on a pending release list and then releasing
the references after dropping the IF_ADDR_LOCK.
- Use the mli_relinmhead list normally used to defer calls to
in6m_release_locked() to defer calls to mld_v1_transmit_report() until
after the IF_ADDR_LOCK is dropped.
Modified:
stable/8/sys/netinet/igmp.c
stable/8/sys/netinet6/in6.c
stable/8/sys/netinet6/mld6.c
Directory Properties:
stable/8/sys/ (props changed)
stable/8/sys/amd64/include/xen/ (props changed)
stable/8/sys/cddl/contrib/opensolaris/ (props changed)
stable/8/sys/contrib/dev/acpica/ (props changed)
stable/8/sys/contrib/pf/ (props changed)
Modified: stable/8/sys/netinet/igmp.c
==============================================================================
--- stable/8/sys/netinet/igmp.c Fri Jan 13 19:50:52 2012 (r230076)
+++ stable/8/sys/netinet/igmp.c Fri Jan 13 19:51:15 2012 (r230077)
@@ -1641,7 +1641,7 @@ igmp_fasttimo_vnet(void)
struct ifqueue qrq; /* Query response packets */
struct ifnet *ifp;
struct igmp_ifinfo *igi;
- struct ifmultiaddr *ifma, *tifma;
+ struct ifmultiaddr *ifma;
struct in_multi *inm;
int loop, uri_fasthz;
@@ -1708,8 +1708,7 @@ igmp_fasttimo_vnet(void)
}
IF_ADDR_LOCK(ifp);
- TAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link,
- tifma) {
+ TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (ifma->ifma_addr->sa_family != AF_INET ||
ifma->ifma_protospec == NULL)
continue;
@@ -2003,7 +2002,7 @@ igmp_v3_cancel_link_timers(struct igmp_i
{
struct ifmultiaddr *ifma;
struct ifnet *ifp;
- struct in_multi *inm;
+ struct in_multi *inm, *tinm;
CTR3(KTR_IGMPV3, "%s: cancel v3 timers on ifp %p(%s)", __func__,
igi->igi_ifp, igi->igi_ifp->if_xname);
@@ -2049,14 +2048,8 @@ igmp_v3_cancel_link_timers(struct igmp_i
* transition to REPORTING to ensure the host leave
* message is sent upstream to the old querier --
* transition to NOT would lose the leave and race.
- *
- * SMPNG: Must drop and re-acquire IF_ADDR_LOCK
- * around inm_release_locked(), as it is not
- * a recursive mutex.
*/
- IF_ADDR_UNLOCK(ifp);
- inm_release_locked(inm);
- IF_ADDR_LOCK(ifp);
+ SLIST_INSERT_HEAD(&igi->igi_relinmhead, inm, inm_nrele);
/* FALLTHROUGH */
case IGMP_G_QUERY_PENDING_MEMBER:
case IGMP_SG_QUERY_PENDING_MEMBER:
@@ -2075,6 +2068,10 @@ igmp_v3_cancel_link_timers(struct igmp_i
_IF_DRAIN(&inm->inm_scq);
}
IF_ADDR_UNLOCK(ifp);
+ SLIST_FOREACH_SAFE(inm, &igi->igi_relinmhead, inm_nrele, tinm) {
+ SLIST_REMOVE_HEAD(&igi->igi_relinmhead, inm_nrele);
+ inm_release_locked(inm);
+ }
}
/*
@@ -3320,7 +3317,7 @@ igmp_v3_merge_state_changes(struct in_mu
static void
igmp_v3_dispatch_general_query(struct igmp_ifinfo *igi)
{
- struct ifmultiaddr *ifma, *tifma;
+ struct ifmultiaddr *ifma;
struct ifnet *ifp;
struct in_multi *inm;
int retval, loop;
@@ -3334,7 +3331,7 @@ igmp_v3_dispatch_general_query(struct ig
ifp = igi->igi_ifp;
IF_ADDR_LOCK(ifp);
- TAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link, tifma) {
+ TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (ifma->ifma_addr->sa_family != AF_INET ||
ifma->ifma_protospec == NULL)
continue;
Modified: stable/8/sys/netinet6/in6.c
==============================================================================
--- stable/8/sys/netinet6/in6.c Fri Jan 13 19:50:52 2012 (r230076)
+++ stable/8/sys/netinet6/in6.c Fri Jan 13 19:51:15 2012 (r230077)
@@ -1195,7 +1195,7 @@ in6_purgeaddr(struct ifaddr *ifa)
struct sockaddr_in6 mask, addr;
int plen, error;
struct rtentry *rt;
- struct ifaddr *ifa0, *nifa;
+ struct ifaddr *ifa0;
/*
* find another IPv6 address as the gateway for the
@@ -1203,7 +1203,7 @@ in6_purgeaddr(struct ifaddr *ifa)
* address routes
*/
IF_ADDR_LOCK(ifp);
- TAILQ_FOREACH_SAFE(ifa0, &ifp->if_addrhead, ifa_link, nifa) {
+ TAILQ_FOREACH(ifa0, &ifp->if_addrhead, ifa_link) {
if ((ifa0->ifa_addr->sa_family != AF_INET6) ||
memcmp(&satosin6(ifa0->ifa_addr)->sin6_addr,
&ia->ia_addr.sin6_addr,
Modified: stable/8/sys/netinet6/mld6.c
==============================================================================
--- stable/8/sys/netinet6/mld6.c Fri Jan 13 19:50:52 2012
(r230076)
+++ stable/8/sys/netinet6/mld6.c Fri Jan 13 19:51:15 2012
(r230077)
@@ -121,7 +121,8 @@ static int mld_v1_input_query(struct ifn
/*const*/ struct mld_hdr *);
static int mld_v1_input_report(struct ifnet *, const struct ip6_hdr *,
/*const*/ struct mld_hdr *);
-static void mld_v1_process_group_timer(struct in6_multi *, const int);
+static void mld_v1_process_group_timer(struct mld_ifinfo *,
+ struct in6_multi *);
static void mld_v1_process_querier_timers(struct mld_ifinfo *);
static int mld_v1_transmit_report(struct in6_multi *, const int);
static void mld_v1_update_group(struct in6_multi *, const int);
@@ -1332,8 +1333,8 @@ mld_fasttimo_vnet(void)
struct ifqueue qrq; /* Query response packets */
struct ifnet *ifp;
struct mld_ifinfo *mli;
- struct ifmultiaddr *ifma, *tifma;
- struct in6_multi *inm;
+ struct ifmultiaddr *ifma;
+ struct in6_multi *inm, *tinm;
int uri_fasthz;
uri_fasthz = 0;
@@ -1397,24 +1398,14 @@ mld_fasttimo_vnet(void)
}
IF_ADDR_LOCK(ifp);
- TAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link,
- tifma) {
+ TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (ifma->ifma_addr->sa_family != AF_INET6 ||
ifma->ifma_protospec == NULL)
continue;
inm = (struct in6_multi *)ifma->ifma_protospec;
switch (mli->mli_version) {
case MLD_VERSION_1:
- /*
- * XXX Drop IF_ADDR lock temporarily to
- * avoid recursion caused by a potential
- * call by in6ifa_ifpforlinklocal().
- * rwlock candidate?
- */
- IF_ADDR_UNLOCK(ifp);
- mld_v1_process_group_timer(inm,
- mli->mli_version);
- IF_ADDR_LOCK(ifp);
+ mld_v1_process_group_timer(mli, inm);
break;
case MLD_VERSION_2:
mld_v2_process_group_timers(mli, &qrq,
@@ -1424,9 +1415,25 @@ mld_fasttimo_vnet(void)
}
IF_ADDR_UNLOCK(ifp);
- if (mli->mli_version == MLD_VERSION_2) {
- struct in6_multi *tinm;
-
+ switch (mli->mli_version) {
+ case MLD_VERSION_1:
+ /*
+ * Transmit reports for this lifecycle. This
+ * is done while not holding IF_ADDR_LOCK
+ * since this can call
+ * in6ifa_ifpforlinklocal() which locks
+ * IF_ADDR_LOCK internally as well as
+ * ip6_output() to transmit a packet.
+ */
+ SLIST_FOREACH_SAFE(inm, &mli->mli_relinmhead,
+ in6m_nrele, tinm) {
+ SLIST_REMOVE_HEAD(&mli->mli_relinmhead,
+ in6m_nrele);
+ (void)mld_v1_transmit_report(inm,
+ MLD_LISTENER_REPORT);
+ }
+ break;
+ case MLD_VERSION_2:
mld_dispatch_queue(&qrq, 0);
mld_dispatch_queue(&scq, 0);
@@ -1440,6 +1447,7 @@ mld_fasttimo_vnet(void)
in6m_nrele);
in6m_release_locked(inm);
}
+ break;
}
}
@@ -1453,7 +1461,7 @@ out_locked:
* Will update the global pending timer flags.
*/
static void
-mld_v1_process_group_timer(struct in6_multi *inm, const int version)
+mld_v1_process_group_timer(struct mld_ifinfo *mli, struct in6_multi *inm)
{
int report_timer_expired;
@@ -1480,8 +1488,8 @@ mld_v1_process_group_timer(struct in6_mu
case MLD_REPORTING_MEMBER:
if (report_timer_expired) {
inm->in6m_state = MLD_IDLE_MEMBER;
- (void)mld_v1_transmit_report(inm,
- MLD_LISTENER_REPORT);
+ SLIST_INSERT_HEAD(&mli->mli_relinmhead, inm,
+ in6m_nrele);
}
break;
case MLD_G_QUERY_PENDING_MEMBER:
@@ -1652,7 +1660,7 @@ mld_v2_cancel_link_timers(struct mld_ifi
{
struct ifmultiaddr *ifma;
struct ifnet *ifp;
- struct in6_multi *inm;
+ struct in6_multi *inm, *tinm;
CTR3(KTR_MLD, "%s: cancel v2 timers on ifp %p(%s)", __func__,
mli->mli_ifp, mli->mli_ifp->if_xname);
@@ -1691,14 +1699,9 @@ mld_v2_cancel_link_timers(struct mld_ifi
* If we are leaving the group and switching
* version, we need to release the final
* reference held for issuing the INCLUDE {}.
- *
- * SMPNG: Must drop and re-acquire IF_ADDR_LOCK
- * around in6m_release_locked(), as it is not
- * a recursive mutex.
*/
- IF_ADDR_UNLOCK(ifp);
- in6m_release_locked(inm);
- IF_ADDR_LOCK(ifp);
+ SLIST_INSERT_HEAD(&mli->mli_relinmhead, inm,
+ in6m_nrele);
/* FALLTHROUGH */
case MLD_G_QUERY_PENDING_MEMBER:
case MLD_SG_QUERY_PENDING_MEMBER:
@@ -1716,6 +1719,10 @@ mld_v2_cancel_link_timers(struct mld_ifi
}
}
IF_ADDR_UNLOCK(ifp);
+ SLIST_FOREACH_SAFE(inm, &mli->mli_relinmhead, in6m_nrele, tinm) {
+ SLIST_REMOVE_HEAD(&mli->mli_relinmhead, in6m_nrele);
+ in6m_release_locked(inm);
+ }
}
/*
@@ -2972,7 +2979,7 @@ mld_v2_merge_state_changes(struct in6_mu
static void
mld_v2_dispatch_general_query(struct mld_ifinfo *mli)
{
- struct ifmultiaddr *ifma, *tifma;
+ struct ifmultiaddr *ifma;
struct ifnet *ifp;
struct in6_multi *inm;
int retval;
@@ -2986,7 +2993,7 @@ mld_v2_dispatch_general_query(struct mld
ifp = mli->mli_ifp;
IF_ADDR_LOCK(ifp);
- TAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link, tifma) {
+ TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (ifma->ifma_addr->sa_family != AF_INET6 ||
ifma->ifma_protospec == NULL)
continue;
_______________________________________________
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"
