mirror/freebsd
1
0
Fork 0
mirror of https://git.FreeBSD.org/src.git synced 2024-12-17 10:26:15 +00:00
Code Issues Releases Activity

Fix in6_multi double free

Browse Source 
This is actually several different bugs:
- The code is not designed to handle inpcb deletion after interface deletion
  - add reference for inpcb membership
- The multicast address has to be removed from interface lists when the refcount
  goes to zero OR when the interface goes away
  - decouple list disconnect from refcount (v6 only for now)
- ifmultiaddr can exist past being on interface lists
  - add flag for tracking whether or not it's enqueued
- deferring freeing moptions makes the incpb cleanup code simpler but opens the
  door wider still to races
  - call inp_gcmoptions synchronously after dropping the the inpcb lock

Fundamentally multicast needs a rewrite - but keep applying band-aids for now.

Tested by: kp
Reported by: novel, kp, lwhsu
This commit is contained in:
Matt Macy 2018-08-15 20:23:08 +00:00
parent 8acbb227d9
commit f9be038601
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=337866
9 changed files with 89 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/net/if.c
View File

@ -3545,6 +3545,7 @@ if_addmulti(struct ifnet *ifp, struct sockaddr *sa,
error = ENOMEM;
goto free_llsa_out;
}
ll_ifma->ifma_flags |= IFMA_F_ENQUEUED;
CK_STAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ll_ifma,
ifma_link);
} else
@ -3557,6 +3558,7 @@ if_addmulti(struct ifnet *ifp, struct sockaddr *sa,
* referenced link layer address. Add the primary address to the
* ifnet address list.
*/
ifma->ifma_flags |= IFMA_F_ENQUEUED;
CK_STAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ifma, ifma_link);
if (retifma != NULL)
@ -3757,9 +3759,10 @@ if_delmulti_locked(struct ifnet *ifp, struct ifmultiaddr *ifma, int detaching)
if (--ifma->ifma_refcount > 0)
return 0;
if (ifp != NULL && detaching == 0)
if (ifp != NULL && detaching == 0 && (ifma->ifma_flags & IFMA_F_ENQUEUED)) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link);
ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
}
/*
* If this ifma is a network-layer ifma, a link-layer ifma may
* have been associated with it. Release it first if so.
@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct ifmultiaddr *ifma, int detaching)
ll_ifma->ifma_ifp = NULL; /* XXX */
if (--ll_ifma->ifma_refcount == 0) {
if (ifp != NULL) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr,
ifma_link);
if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr,
ifma_link);
ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
}
}
if_freemulti(ll_ifma);
}

2
sys/net/if_var.h
View File

@ -548,12 +548,14 @@ void ifa_ref(struct ifaddr *ifa);
* Multicast address structure. This is analogous to the ifaddr
* structure except that it keeps track of multicast addresses.
*/
#define IFMA_F_ENQUEUED 0x1
struct ifmultiaddr {
CK_STAILQ_ENTRY(ifmultiaddr) ifma_link; /* queue macro glue */
struct sockaddr *ifma_addr; /* address this membership is for */
struct sockaddr *ifma_lladdr; /* link-layer translation, if any */
struct ifnet *ifma_ifp; /* back-pointer to interface */
u_int ifma_refcount; /* reference count */
int ifma_flags;
void *ifma_protospec; /* protocol-specific state, if any */
struct ifmultiaddr *ifma_llifma; /* pointer to ifma for ifma_lladdr */
struct epoch_context ifma_epoch_ctx;

26
sys/netinet/in_mcast.c
View File

@ -263,7 +263,10 @@ inm_disconnect(struct in_multi *inm)
ifma = inm->inm_ifma;
if_ref(ifp);
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link);
if (ifma->ifma_flags & IFMA_F_ENQUEUED) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link);
ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
}
MCDPRINTF("removed ifma: %p from %s\n", ifma, ifp->if_xname);
if ((ll_ifma = ifma->ifma_llifma) != NULL) {
MPASS(ifma != ll_ifma);
@ -271,7 +274,10 @@ inm_disconnect(struct in_multi *inm)
MPASS(ll_ifma->ifma_llifma == NULL);
MPASS(ll_ifma->ifma_ifp == ifp);
if (--ll_ifma->ifma_refcount == 0) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr, ifma_link);
if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr, ifma_link);
ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
}
MCDPRINTF("removed ll_ifma: %p from %s\n", ll_ifma, ifp->if_xname);
if_freemulti(ll_ifma);
ifma_restart = true;
@ -1668,16 +1674,13 @@ inp_findmoptions(struct inpcb *inp)
}
static void
inp_gcmoptions(epoch_context_t ctx)
inp_gcmoptions(struct ip_moptions *imo)
{
struct ip_moptions *imo;
struct in_mfilter *imf;
struct in_multi *inm;
struct ifnet *ifp;
size_t idx, nmships;
imo = __containerof(ctx, struct ip_moptions, imo_epoch_ctx);
nmships = imo->imo_num_memberships;
for (idx = 0; idx < nmships; ++idx) {
imf = imo->imo_mfilters ? &imo->imo_mfilters[idx] : NULL;
@ -1713,7 +1716,7 @@ inp_freemoptions(struct ip_moptions *imo)
{
if (imo == NULL)
return;
epoch_call(net_epoch_preempt, &imo->imo_epoch_ctx, inp_gcmoptions);
inp_gcmoptions(imo);
}
/*
@ -2265,7 +2268,8 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt)
__func__);
IN_MULTI_LIST_UNLOCK();
goto out_imo_free;
}
}
inm_acquire(inm);
imo->imo_membership[idx] = inm;
} else {
CTR1(KTR_IGMPV3, "%s: merge inm state", __func__);
@ -2305,6 +2309,12 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt)
out_imo_free:
if (error && is_new) {
inm = imo->imo_membership[idx];
if (inm != NULL) {
IN_MULTI_LIST_LOCK();
inm_release_deferred(inm);
IN_MULTI_LIST_UNLOCK();
}
imo->imo_membership[idx] = NULL;
--imo->imo_num_memberships;
}

11
sys/netinet/in_pcb.c
View File

@ -1578,7 +1578,7 @@ in_pcbfree_deferred(epoch_context_t ctx)
INP_WLOCK(inp);
#ifdef INET
inp_freemoptions(inp->inp_moptions);
struct ip_moptions *imo = inp->inp_moptions;
inp->inp_moptions = NULL;
#endif
/* XXXRW: Do as much as possible here. */
@ -1587,9 +1587,10 @@ in_pcbfree_deferred(epoch_context_t ctx)
ipsec_delete_pcbpolicy(inp);
#endif
#ifdef INET6
struct ip6_moptions *im6o = NULL;
if (inp->inp_vflag & INP_IPV6PROTO) {
ip6_freepcbopts(inp->in6p_outputopts);
ip6_freemoptions(inp->in6p_moptions);
im6o = inp->in6p_moptions;
inp->in6p_moptions = NULL;
}
#endif
@ -1602,6 +1603,12 @@ in_pcbfree_deferred(epoch_context_t ctx)
#endif
released = in_pcbrele_wlocked(inp);
MPASS(released);
#ifdef INET6
ip6_freemoptions(im6o);
#endif
#ifdef INET
inp_freemoptions(imo);
#endif
}
/*

2
sys/netinet/ip_carp.c
View File

@ -1423,6 +1423,7 @@ carp_multicast_setup(struct carp_if *cif, sa_family_t sa)
free(im6o->im6o_membership, M_CARP);
break;
}
in6m_acquire(in6m);
im6o->im6o_membership[0] = in6m;
im6o->im6o_num_memberships++;
@ -1444,6 +1445,7 @@ carp_multicast_setup(struct carp_if *cif, sa_family_t sa)
free(im6o->im6o_membership, M_CARP);
break;
}
in6m_acquire(in6m);
im6o->im6o_membership[1] = in6m;
im6o->im6o_num_memberships++;
break;

1
sys/netinet6/in6_ifattach.c
View File

@ -863,6 +863,7 @@ in6_purgemaddrs(struct ifnet *ifp)
ifma->ifma_protospec == NULL)
continue;
inm = (struct in6_multi *)ifma->ifma_protospec;
in6m_disconnect(inm);
in6m_rele_locked(&purgeinms, inm);
if (__predict_false(ifma6_restart)) {
ifma6_restart = false;

41
sys/netinet6/in6_mcast.c
View File

@ -538,6 +538,8 @@ in6m_release(struct in6_multi *inm)
CTR2(KTR_MLD, "%s: purging ifma %p", __func__, ifma);
KASSERT(ifma->ifma_protospec == NULL,
("%s: ifma_protospec != NULL", __func__));
if (ifp == NULL)
ifp = ifma->ifma_ifp;
if (ifp != NULL) {
CURVNET_SET(ifp->if_vnet);
@ -592,11 +594,20 @@ in6m_disconnect(struct in6_multi *inm)
struct ifmultiaddr *ifma, *ll_ifma;
ifp = inm->in6m_ifp;
if (ifp == NULL)
return;
inm->in6m_ifp = NULL;
IF_ADDR_WLOCK_ASSERT(ifp);
ifma = inm->in6m_ifma;
if (ifma == NULL)
return;
if_ref(ifp);
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link);
if (ifma->ifma_flags & IFMA_F_ENQUEUED) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ifma, ifmultiaddr, ifma_link);
ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
}
MCDPRINTF("removed ifma: %p from %s\n", ifma, ifp->if_xname);
if ((ll_ifma = ifma->ifma_llifma) != NULL) {
MPASS(ifma != ll_ifma);
@ -605,7 +616,10 @@ in6m_disconnect(struct in6_multi *inm)
MPASS(ll_ifma->ifma_ifp == ifp);
if (--ll_ifma->ifma_refcount == 0) {
ifma6_restart = true;
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr, ifma_link);
if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED) {
CK_STAILQ_REMOVE(&ifp->if_multiaddrs, ll_ifma, ifmultiaddr, ifma_link);
ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
}
MCDPRINTF("removed ll_ifma: %p from %s\n", ll_ifma, ifp->if_xname);
if_freemulti(ll_ifma);
}
@ -632,7 +646,7 @@ in6m_release_deferred(struct in6_multi *inm)
IN6_MULTI_LIST_LOCK_ASSERT();
KASSERT(inm->in6m_refcount > 0, ("refcount == %d inm: %p", inm->in6m_refcount, inm));
if (--inm->in6m_refcount == 0) {
in6m_disconnect(inm);
MPASS(inm->in6m_ifp == NULL);
SLIST_INIT(&tmp);
inm->in6m_ifma->ifma_protospec = NULL;
MPASS(inm->in6m_ifma->ifma_llifma == NULL);
@ -1310,6 +1324,7 @@ in6_joingroup_locked(struct ifnet *ifp, const struct in6_addr *mcaddr,
break;
}
}
in6m_disconnect(inm);
in6m_release_deferred(inm);
IF_ADDR_RUNLOCK(ifp);
} else {
@ -1389,13 +1404,17 @@ in6_leavegroup_locked(struct in6_multi *inm, /*const*/ struct in6_mfilter *imf)
KASSERT(error == 0, ("%s: failed to merge inm state", __func__));
CTR1(KTR_MLD, "%s: doing mld downcall", __func__);
error = mld_change_state(inm, 0);
error = 0;
if (ifp)
error = mld_change_state(inm, 0);
if (error)
CTR1(KTR_MLD, "%s: failed mld downcall", __func__);
CTR2(KTR_MLD, "%s: dropping ref on %p", __func__, inm);
if (ifp)
IF_ADDR_WLOCK(ifp);
if (inm->in6m_refcount == 1 && inm->in6m_ifp != NULL)
in6m_disconnect(inm);
in6m_release_deferred(inm);
if (ifp)
IF_ADDR_WUNLOCK(ifp);
@ -1629,16 +1648,13 @@ in6p_findmoptions(struct inpcb *inp)
*/
static void
inp_gcmoptions(epoch_context_t ctx)
inp_gcmoptions(struct ip6_moptions *imo)
{
struct ip6_moptions *imo;
struct in6_mfilter *imf;
struct in6_multi *inm;
struct ifnet *ifp;
size_t idx, nmships;
imo = __containerof(ctx, struct ip6_moptions, imo6_epoch_ctx);
nmships = imo->im6o_num_memberships;
for (idx = 0; idx < nmships; ++idx) {
imf = imo->im6o_mfilters ? &imo->im6o_mfilters[idx] : NULL;
@ -1668,7 +1684,7 @@ ip6_freemoptions(struct ip6_moptions *imo)
{
if (imo == NULL)
return;
epoch_call(net_epoch_preempt, &imo->imo6_epoch_ctx, inp_gcmoptions);
inp_gcmoptions(imo);
}
/*
@ -2162,6 +2178,7 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sopt)
IN6_MULTI_UNLOCK();
goto out_im6o_free;
}
in6m_acquire(inm);
imo->im6o_membership[idx] = inm;
} else {
CTR1(KTR_MLD, "%s: merge inm state", __func__);
@ -2196,6 +2213,12 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sopt)
out_im6o_free:
if (error && is_new) {
inm = imo->im6o_membership[idx];
if (inm != NULL) {
IN6_MULTI_LIST_LOCK();
in6m_release_deferred(inm);
IN6_MULTI_LIST_UNLOCK();
}
imo->im6o_membership[idx] = NULL;
--imo->im6o_num_memberships;
}

2
sys/netinet6/in6_var.h
View File

@ -784,7 +784,7 @@ in6m_rele_locked(struct in6_multi_head *inmh, struct in6_multi *inm)
IN6_MULTI_LIST_LOCK_ASSERT();
if (--inm->in6m_refcount == 0) {
in6m_disconnect(inm);
MPASS(inm->in6m_ifp == NULL);
inm->in6m_ifma->ifma_protospec = NULL;
MPASS(inm->in6m_ifma->ifma_llifma == NULL);
SLIST_INSERT_HEAD(inmh, inm, in6m_nrele);

25
sys/netinet6/mld6.c
View File

@ -557,6 +557,7 @@ mld_ifdetach(struct ifnet *ifp)
continue;
inm = (struct in6_multi *)ifma->ifma_protospec;
if (inm->in6m_state == MLD_LEAVING_MEMBER) {
in6m_disconnect(inm);
in6m_rele_locked(&inmh, inm);
ifma->ifma_protospec = NULL;
}
@ -1483,6 +1484,7 @@ mld_v1_process_group_timer(struct in6_multi_head *inmh, struct in6_multi *inm)
case MLD_REPORTING_MEMBER:
if (report_timer_expired) {
inm->in6m_state = MLD_IDLE_MEMBER;
in6m_disconnect(inm);
in6m_rele_locked(inmh, inm);
}
break;
@ -1607,6 +1609,7 @@ mld_v2_process_group_timers(struct in6_multi_head *inmh,
if (inm->in6m_state == MLD_LEAVING_MEMBER &&
inm->in6m_scrv == 0) {
inm->in6m_state = MLD_NOT_MEMBER;
in6m_disconnect(inm);
in6m_rele_locked(inmh, inm);
}
}
@ -1697,6 +1700,7 @@ mld_v2_cancel_link_timers(struct mld_ifsoftc *mli)
* version, we need to release the final
* reference held for issuing the INCLUDE {}.
*/
in6m_disconnect(inm);
in6m_rele_locked(&inmh, inm);
ifma->ifma_protospec = NULL;
/* FALLTHROUGH */
@ -1893,16 +1897,15 @@ mld_change_state(struct in6_multi *inm, const int delay)
*/
KASSERT(inm->in6m_ifma != NULL, ("%s: no ifma", __func__));
ifp = inm->in6m_ifma->ifma_ifp;
if (ifp != NULL) {
/*
* Sanity check that netinet6's notion of ifp is the
* same as net's.
*/
KASSERT(inm->in6m_ifp == ifp, ("%s: bad ifp", __func__));
}
if (ifp == NULL)
return (0);
/*
* Sanity check that netinet6's notion of ifp is the
* same as net's.
*/
KASSERT(inm->in6m_ifp == ifp, ("%s: bad ifp", __func__));
MLD_LOCK();
mli = MLD_IFINFO(ifp);
KASSERT(mli != NULL, ("%s: no mld_ifsoftc for ifp %p", __func__, ifp));
@ -1996,9 +1999,9 @@ mld_initial_join(struct in6_multi *inm, struct mld_ifsoftc *mli,
* group around for the final INCLUDE {} enqueue.
*/
if (mli->mli_version == MLD_VERSION_2 &&
inm->in6m_state == MLD_LEAVING_MEMBER)
in6m_release_deferred(inm);
inm->in6m_state == MLD_LEAVING_MEMBER) {
inm->in6m_refcount--;
}
inm->in6m_state = MLD_REPORTING_MEMBER;
switch (mli->mli_version) {