Changeset View
Changeset View
Standalone View
Standalone View
sys/netinet/in_mcast.c
Show First 20 Lines • Show All 260 Lines • ▼ Show 20 Lines | inm_disconnect(struct in_multi *inm) | ||||
ifp = inm->inm_ifp; | ifp = inm->inm_ifp; | ||||
IF_ADDR_WLOCK_ASSERT(ifp); | IF_ADDR_WLOCK_ASSERT(ifp); | ||||
ifma = inm->inm_ifma; | ifma = inm->inm_ifma; | ||||
if_ref(ifp); | if_ref(ifp); | ||||
if (ifma->ifma_flags & IFMA_F_ENQUEUED) { | if (ifma->ifma_flags & IFMA_F_ENQUEUED) { | ||||
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link); | CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link); | ||||
ifma->ifma_flags &= ~IFMA_F_ENQUEUED; | ifma->ifma_flags &= ~IFMA_F_ENQUEUED; | ||||
} | |||||
MCDPRINTF("removed ifma: %p from %s\n", ifma, ifp->if_xname); | MCDPRINTF("removed ifma: %p from %s\n", ifma, ifp->if_xname); | ||||
if ((ll_ifma = ifma->ifma_llifma) != NULL) { | } | ||||
if (ifma->ifma_ifp != NULL && (ll_ifma = ifma->ifma_llifma) != NULL) { | |||||
MPASS(ifma != ll_ifma); | MPASS(ifma != ll_ifma); | ||||
ifma->ifma_llifma = NULL; | ifma->ifma_llifma = NULL; | ||||
MPASS(ll_ifma->ifma_llifma == NULL); | MPASS(ll_ifma->ifma_llifma == NULL); | ||||
MPASS(ll_ifma->ifma_ifp == ifp); | MPASS(ll_ifma->ifma_ifp == ifp); | ||||
if (--ll_ifma->ifma_refcount == 0) { | if (--ll_ifma->ifma_refcount == 0) { | ||||
if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED) { | if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED) { | ||||
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr, ifma_link); | CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr, ifma_link); | ||||
ll_ifma->ifma_flags &= ~IFMA_F_ENQUEUED; | ll_ifma->ifma_flags &= ~IFMA_F_ENQUEUED; | ||||
} | |||||
MCDPRINTF("removed ll_ifma: %p from %s\n", ll_ifma, ifp->if_xname); | MCDPRINTF("removed ll_ifma: %p from %s\n", ll_ifma, ifp->if_xname); | ||||
} | |||||
if_freemulti(ll_ifma); | if_freemulti(ll_ifma); | ||||
ifma_restart = true; | ifma_restart = true; | ||||
} | } | ||||
} | } | ||||
} | } | ||||
void | void | ||||
inm_release_deferred(struct in_multi *inm) | inm_release_deferred(struct in_multi *inm) | ||||
{ | { | ||||
struct in_multi_head tmp; | struct in_multi_head tmp; | ||||
IN_MULTI_LIST_LOCK_ASSERT(); | IN_MULTI_LIST_LOCK_ASSERT(); | ||||
MPASS(inm->inm_refcount > 0); | MPASS(inm->inm_refcount > 0); | ||||
MCDPRINTF("inm: %p refcount: %d\n", inm, inm->inm_refcount); | |||||
if (--inm->inm_refcount == 0) { | if (--inm->inm_refcount == 0) { | ||||
MCDPRINTF("freeing %p \n", inm); | |||||
SLIST_INIT(&tmp); | SLIST_INIT(&tmp); | ||||
inm_disconnect(inm); | inm_disconnect(inm); | ||||
inm->inm_ifma->ifma_protospec = NULL; | inm->inm_ifma->ifma_protospec = NULL; | ||||
SLIST_INSERT_HEAD(&tmp, inm, inm_nrele); | SLIST_INSERT_HEAD(&tmp, inm, inm_nrele); | ||||
inm_release_list_deferred(&tmp); | inm_release_list_deferred(&tmp); | ||||
} | } | ||||
} | } | ||||
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines | CK_STAILQ_FOREACH(ifma, &((ifp)->if_multiaddrs), ifma_link) { | ||||
if (ifma->ifma_addr->sa_family != AF_INET || | if (ifma->ifma_addr->sa_family != AF_INET || | ||||
ifma->ifma_protospec == NULL) | ifma->ifma_protospec == NULL) | ||||
continue; | continue; | ||||
inm = (struct in_multi *)ifma->ifma_protospec; | inm = (struct in_multi *)ifma->ifma_protospec; | ||||
if (inm->inm_addr.s_addr == ina.s_addr) | if (inm->inm_addr.s_addr == ina.s_addr) | ||||
break; | break; | ||||
inm = NULL; | inm = NULL; | ||||
} | } | ||||
if (inm) | |||||
inm_acquire_locked(inm); | |||||
return (inm); | return (inm); | ||||
} | } | ||||
/* | /* | ||||
* Wrapper for inm_lookup_locked(). | * Wrapper for inm_lookup_locked(). | ||||
* The IF_ADDR_LOCK will be taken on ifp and released on return. | * The IF_ADDR_LOCK will be taken on ifp and released on return. | ||||
*/ | */ | ||||
struct in_multi * | struct in_multi * | ||||
▲ Show 20 Lines • Show All 187 Lines • ▼ Show 20 Lines | in_getmulti(struct ifnet *ifp, const struct in_addr *group, | ||||
int error; | int error; | ||||
IN_MULTI_LOCK_ASSERT(); | IN_MULTI_LOCK_ASSERT(); | ||||
ii = (struct in_ifinfo *)ifp->if_afdata[AF_INET]; | ii = (struct in_ifinfo *)ifp->if_afdata[AF_INET]; | ||||
IN_MULTI_LIST_LOCK(); | IN_MULTI_LIST_LOCK(); | ||||
inm = inm_lookup(ifp, *group); | inm = inm_lookup(ifp, *group); | ||||
if (inm != NULL) { | if (inm != NULL) { | ||||
/* | KASSERT(inm->inm_refcount > 1, | ||||
hselasky: Nothing is preventing a lookup with inm_refcount == 1, which then will assert ?? | |||||
Done Inline ActionsIn order to be found in on a list it has to have a reference for that list in addition to the caller. The assert could only falsely trigger if the lock weren't held at the time and you hit a race. mmacy: In order to be found in on a list it has to have a reference for that list in addition to the… | |||||
* If we already joined this group, just bump the | |||||
* refcount and return it. | |||||
*/ | |||||
KASSERT(inm->inm_refcount >= 1, | |||||
("%s: bad refcount %d", __func__, inm->inm_refcount)); | ("%s: bad refcount %d", __func__, inm->inm_refcount)); | ||||
inm_acquire_locked(inm); | |||||
*pinm = inm; | *pinm = inm; | ||||
} | } | ||||
IN_MULTI_LIST_UNLOCK(); | IN_MULTI_LIST_UNLOCK(); | ||||
if (inm != NULL) | if (inm != NULL) | ||||
return (0); | return (0); | ||||
memset(&gsin, 0, sizeof(gsin)); | memset(&gsin, 0, sizeof(gsin)); | ||||
gsin.sin_family = AF_INET; | gsin.sin_family = AF_INET; | ||||
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Lines | if (inm == NULL) { | ||||
IN_MULTI_LIST_UNLOCK(); | IN_MULTI_LIST_UNLOCK(); | ||||
if_delmulti_ifma(ifma); | if_delmulti_ifma(ifma); | ||||
return (ENOMEM); | return (ENOMEM); | ||||
} | } | ||||
inm->inm_addr = *group; | inm->inm_addr = *group; | ||||
inm->inm_ifp = ifp; | inm->inm_ifp = ifp; | ||||
inm->inm_igi = ii->ii_igmp; | inm->inm_igi = ii->ii_igmp; | ||||
inm->inm_ifma = ifma; | inm->inm_ifma = ifma; | ||||
inm->inm_refcount = 1; | ifma->ifma_refcount++; | ||||
/* One reference for the caller and one for the ifma */ | |||||
Not Done Inline Actionsin6_getmulti() is only setting refcount = 1 .... Can you try to follow the approach taken there? hselasky: in6_getmulti() is only setting refcount = 1 .... Can you try to follow the approach taken… | |||||
Done Inline ActionsNo. This is a bug. It relies on the caller to differentiate between when it already exists and when it's new. mmacy: No. This is a bug. It relies on the caller to differentiate between when it already exists and… | |||||
inm->inm_refcount = 2; | |||||
inm->inm_state = IGMP_NOT_MEMBER; | inm->inm_state = IGMP_NOT_MEMBER; | ||||
mbufq_init(&inm->inm_scq, IGMP_MAX_STATE_CHANGES); | mbufq_init(&inm->inm_scq, IGMP_MAX_STATE_CHANGES); | ||||
inm->inm_st[0].iss_fmode = MCAST_UNDEFINED; | inm->inm_st[0].iss_fmode = MCAST_UNDEFINED; | ||||
inm->inm_st[1].iss_fmode = MCAST_UNDEFINED; | inm->inm_st[1].iss_fmode = MCAST_UNDEFINED; | ||||
RB_INIT(&inm->inm_srcs); | RB_INIT(&inm->inm_srcs); | ||||
ifma->ifma_protospec = inm; | ifma->ifma_protospec = inm; | ||||
▲ Show 20 Lines • Show All 659 Lines • ▼ Show 20 Lines | if (error) { | ||||
CTR1(KTR_IGMPV3, "%s: failed to update source", __func__); | CTR1(KTR_IGMPV3, "%s: failed to update source", __func__); | ||||
goto out_inm_release; | goto out_inm_release; | ||||
} | } | ||||
out_inm_release: | out_inm_release: | ||||
if (error) { | if (error) { | ||||
CTR2(KTR_IGMPV3, "%s: dropping ref on %p", __func__, inm); | CTR2(KTR_IGMPV3, "%s: dropping ref on %p", __func__, inm); | ||||
/* drop the callers reference */ | |||||
inm_release_deferred(inm); | inm_release_deferred(inm); | ||||
} else { | } else { | ||||
*pinm = inm; | *pinm = inm; | ||||
} | } | ||||
IN_MULTI_LIST_UNLOCK(); | IN_MULTI_LIST_UNLOCK(); | ||||
return (error); | return (error); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 890 Lines • ▼ Show 20 Lines | inp_join_group(struct inpcb *inp, struct sockopt *sopt) | ||||
INP_WLOCK_ASSERT(inp); | INP_WLOCK_ASSERT(inp); | ||||
if (is_new) { | if (is_new) { | ||||
if (imo->imo_num_memberships == imo->imo_max_memberships) { | if (imo->imo_num_memberships == imo->imo_max_memberships) { | ||||
error = imo_grow(imo); | error = imo_grow(imo); | ||||
if (error) | if (error) | ||||
goto out_inp_locked; | goto out_inp_locked; | ||||
} | } | ||||
/* | |||||
* Allocate the new slot upfront so we can deal with | |||||
* grafting the new source filter in same code path | |||||
* as for join-source on existing membership. | |||||
*/ | |||||
idx = imo->imo_num_memberships; | idx = imo->imo_num_memberships; | ||||
imo->imo_membership[idx] = NULL; | imo->imo_membership[idx] = NULL; | ||||
imo->imo_num_memberships++; | |||||
KASSERT(imo->imo_mfilters != NULL, | KASSERT(imo->imo_mfilters != NULL, | ||||
("%s: imf_mfilters vector was not allocated", __func__)); | ("%s: imf_mfilters vector was not allocated", __func__)); | ||||
imf = &imo->imo_mfilters[idx]; | imf = &imo->imo_mfilters[idx]; | ||||
KASSERT(RB_EMPTY(&imf->imf_sources), | KASSERT(RB_EMPTY(&imf->imf_sources), | ||||
("%s: imf_sources not empty", __func__)); | ("%s: imf_sources not empty", __func__)); | ||||
} | } | ||||
/* | /* | ||||
Show All 36 Lines | inp_join_group(struct inpcb *inp, struct sockopt *sopt) | ||||
in_pcbref(inp); | in_pcbref(inp); | ||||
INP_WUNLOCK(inp); | INP_WUNLOCK(inp); | ||||
IN_MULTI_LOCK(); | IN_MULTI_LOCK(); | ||||
if (is_new) { | if (is_new) { | ||||
error = in_joingroup_locked(ifp, &gsa->sin.sin_addr, imf, | error = in_joingroup_locked(ifp, &gsa->sin.sin_addr, imf, | ||||
&inm); | &inm); | ||||
if (error) { | if (error) { | ||||
CTR1(KTR_IGMPV3, "%s: in_joingroup_locked failed", | CTR1(KTR_IGMPV3, "%s: in_joingroup_locked failed", | ||||
__func__); | __func__); | ||||
IN_MULTI_LIST_UNLOCK(); | goto out_in_multi_locked; | ||||
goto out_imo_free; | |||||
} | } | ||||
inm_acquire(inm); | /* joingroup returns with a reference held */ | ||||
Not Done Inline ActionsPlease add a comment describing that the refcount is already acquired. hselasky: Please add a comment describing that the refcount is already acquired. | |||||
imo->imo_membership[idx] = inm; | imo->imo_membership[idx] = inm; | ||||
MPASS(idx == imo->imo_num_memberships); | |||||
imo->imo_num_memberships++; | |||||
} else { | } else { | ||||
CTR1(KTR_IGMPV3, "%s: merge inm state", __func__); | CTR1(KTR_IGMPV3, "%s: merge inm state", __func__); | ||||
IN_MULTI_LIST_LOCK(); | IN_MULTI_LIST_LOCK(); | ||||
error = inm_merge(inm, imf); | error = inm_merge(inm, imf); | ||||
if (error) { | if (error) { | ||||
CTR1(KTR_IGMPV3, "%s: failed to merge inm state", | CTR1(KTR_IGMPV3, "%s: failed to merge inm state", | ||||
__func__); | __func__); | ||||
IN_MULTI_LIST_UNLOCK(); | IN_MULTI_LIST_UNLOCK(); | ||||
goto out_in_multi_locked; | goto out_in_multi_locked; | ||||
} | } | ||||
CTR1(KTR_IGMPV3, "%s: doing igmp downcall", __func__); | CTR1(KTR_IGMPV3, "%s: doing igmp downcall", __func__); | ||||
error = igmp_change_state(inm); | error = igmp_change_state(inm); | ||||
IN_MULTI_LIST_UNLOCK(); | IN_MULTI_LIST_UNLOCK(); | ||||
if (error) { | if (error) { | ||||
CTR1(KTR_IGMPV3, "%s: failed igmp downcall", | CTR1(KTR_IGMPV3, "%s: failed igmp downcall", | ||||
__func__); | __func__); | ||||
goto out_in_multi_locked; | goto out_in_multi_locked; | ||||
} | } | ||||
} | } | ||||
out_in_multi_locked: | out_in_multi_locked: | ||||
if (imo->imo_num_memberships) | |||||
MPASS(imo->imo_membership[0]); | |||||
IN_MULTI_UNLOCK(); | IN_MULTI_UNLOCK(); | ||||
INP_WLOCK(inp); | INP_WLOCK(inp); | ||||
if (in_pcbrele_wlocked(inp)) | if (in_pcbrele_wlocked(inp)) | ||||
return (ENXIO); | return (ENXIO); | ||||
if (error) { | if (error) { | ||||
imf_rollback(imf); | imf_rollback(imf); | ||||
if (is_new) | if (is_new) | ||||
imf_purge(imf); | imf_purge(imf); | ||||
else | else | ||||
imf_reap(imf); | imf_reap(imf); | ||||
} else { | } else { | ||||
imf_commit(imf); | imf_commit(imf); | ||||
} | } | ||||
out_imo_free: | out_imo_free: | ||||
if (error && is_new) { | if (error && is_new) { | ||||
inm = imo->imo_membership[idx]; | inm = imo->imo_membership[idx]; | ||||
if (inm != NULL) { | if (inm != NULL) { | ||||
IN_MULTI_LIST_LOCK(); | IN_MULTI_LIST_LOCK(); | ||||
inm_release_deferred(inm); | inm_release_deferred(inm); | ||||
IN_MULTI_LIST_UNLOCK(); | IN_MULTI_LIST_UNLOCK(); | ||||
} | } | ||||
imo->imo_membership[idx] = NULL; | imo->imo_membership[idx] = NULL; | ||||
--imo->imo_num_memberships; | |||||
} | } | ||||
out_inp_locked: | out_inp_locked: | ||||
INP_WUNLOCK(inp); | INP_WUNLOCK(inp); | ||||
return (error); | return (error); | ||||
} | } | ||||
/* | /* | ||||
▲ Show 20 Lines • Show All 808 Lines • Show Last 20 Lines |
Nothing is preventing a lookup with inm_refcount == 1, which then will assert ??