diff options
| author | jhb <jhb@FreeBSD.org> | 2012-01-03 20:34:52 +0000 |
|---|---|---|
| committer | jhb <jhb@FreeBSD.org> | 2012-01-03 20:34:52 +0000 |
| commit | 66d3d3405cffb0025c47c185520370dcbe69992e (patch) | |
| tree | d2697b117f50915c6220d487fb3e903bb74e0f33 /sys/netinet/igmp.c | |
| parent | 5630bbccbea14cd45f922883709058564c8cd76a (diff) | |
| download | FreeBSD-src-66d3d3405cffb0025c47c185520370dcbe69992e.zip FreeBSD-src-66d3d3405cffb0025c47c185520370dcbe69992e.tar.gz | |
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
Diffstat (limited to 'sys/netinet/igmp.c')
| -rw-r--r-- | sys/netinet/igmp.c | 14 |
1 files changed, 6 insertions, 8 deletions
diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c index 79f6db5..3561fb8 100644 --- a/sys/netinet/igmp.c +++ b/sys/netinet/igmp.c @@ -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); + } } /* |
