Author: jhb
Date: Tue Jan 3 20:34:52 2012
New Revision: 229420
URL: http://svn.freebsd.org/changeset/base/229420
Log:
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 pending release list and then releasing
the references after dropping the IF_ADDR_LOCK. This closes an ugly race
where the code was dropping the lock in the middle of iterating over the
list. It also fixes some additional potential use-after-free bugs since
the cancellation routine also applied other changes to the group after
dropping the reference. Now those changes are performed before the
reference is dropped and the group is potentially freed.
Prodded to fix by: glebius
Reviewed by: bz
MFC after: 1 week
Modified:
head/sys/netinet/igmp.c
head/sys/netinet6/mld6.c
Modified: head/sys/netinet/igmp.c
==============================================================================
--- head/sys/netinet/igmp.c Tue Jan 3 20:28:17 2012 (r229419)
+++ head/sys/netinet/igmp.c Tue Jan 3 20:34:52 2012 (r229420)
@@ -2003,7 +2003,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 +2049,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 +2069,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);
+ }
}
/*
Modified: head/sys/netinet6/mld6.c
==============================================================================
--- head/sys/netinet6/mld6.c Tue Jan 3 20:28:17 2012 (r229419)
+++ head/sys/netinet6/mld6.c Tue Jan 3 20:34:52 2012 (r229420)
@@ -1656,7 +1656,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);
@@ -1695,14 +1695,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:
@@ -1720,6 +1715,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);
+ }
}
/*
_______________________________________________
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"
