mirror of
https://github.com/freebsd/freebsd-src
synced 2024-10-17 13:53:39 +00:00
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
This commit is contained in:
parent
3093cb15fb
commit
c6f4ea8062
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=229420
|
@ -2003,7 +2003,7 @@ igmp_v3_cancel_link_timers(struct igmp_ifinfo *igi)
|
|||
{
|
||||
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_ifinfo *igi)
|
|||
* 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_ifinfo *igi)
|
|||
_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);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
@ -1656,7 +1656,7 @@ mld_v2_cancel_link_timers(struct mld_ifinfo *mli)
|
|||
{
|
||||
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_ifinfo *mli)
|
|||
* 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_ifinfo *mli)
|
|||
}
|
||||
}
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
Loading…
Reference in a new issue