Attachment 200956 Details for Bug 233535 – Fix MLD refcounting in IPv6 code.

[patch] Fix MLD refcounting in IPv6 code.

in6_multi.diff (text/plain), 5.15 KB, created by Hans Petter Selasky on 2019-01-09 14:46:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Hans Petter Selasky

Created: 2019-01-09 14:46:06 UTC

Size: 5.15 KB

patch

obsolete

>Index: sys/netinet6/in6_ifattach.c
>===================================================================
>--- sys/netinet6/in6_ifattach.c	(revision 342455)
>+++ sys/netinet6/in6_ifattach.c	(working copy)
>@@ -870,7 +870,7 @@
> 		    ifma->ifma_protospec == NULL)
> 			continue;
> 		inm = (struct in6_multi *)ifma->ifma_protospec;
>-		in6m_disconnect(inm);
>+		in6m_disconnect_locked(&purgeinms, inm);
> 		in6m_rele_locked(&purgeinms, inm);
> 		if (__predict_false(ifma6_restart)) {
> 			ifma6_restart = false;
>@@ -882,6 +882,13 @@
> 	IN6_MULTI_LIST_UNLOCK();
> 	IN6_MULTI_UNLOCK();
> 	in6m_release_list_deferred(&purgeinms);
>+
>+	/*
>+	 * Make sure all multicast deletions invoking if_ioctl() are
>+	 * completed before returning. Else we risk accessing a freed
>+	 * ifnet structure pointer.
>+	 */
>+	in6m_release_wait();
> }
> 
> void
>Index: sys/netinet6/in6_mcast.c
>===================================================================
>--- sys/netinet6/in6_mcast.c	(revision 342455)
>+++ sys/netinet6/in6_mcast.c	(working copy)
>@@ -585,8 +585,15 @@
> }
> 
> void
>-in6m_disconnect(struct in6_multi *inm)
>+in6m_release_wait(void)
> {
>+	/* wait for all jobs to complete */
>+	gtaskqueue_drain_all(free_gtask.gt_taskqueue);
>+}
>+
>+void
>+in6m_disconnect_locked(struct in6_multi_head *inmh, struct in6_multi *inm)
>+{
> 	struct ifnet *ifp;
> 	struct ifaddr *ifa;
> 	struct in6_ifaddr *ifa6;
>@@ -593,10 +600,12 @@
> 	struct in6_multi_mship *imm, *imm_tmp;
> 	struct ifmultiaddr *ifma, *ll_ifma;
> 
>+	IN6_MULTI_LIST_LOCK_ASSERT();
>+
> 	ifp = inm->in6m_ifp;
>+	if (ifp == NULL)
>+		return;		/* already called */
> 
>-	if (ifp == NULL)
>-		return;
> 	inm->in6m_ifp = NULL;
> 	IF_ADDR_WLOCK_ASSERT(ifp);
> 	ifma = inm->in6m_ifma;
>@@ -633,6 +642,7 @@
> 			if (inm == imm->i6mm_maddr) {
> 				LIST_REMOVE(imm, i6mm_chain);
> 				free(imm, M_IP6MADDR);
>+				in6m_rele_locked(inmh, inm);
> 			}
> 		}
> 	}
>@@ -1324,7 +1334,7 @@
> 				break;
> 			}
> 		}
>-		in6m_disconnect(inm);
>+		in6m_disconnect_locked(NULL, inm);
> 		in6m_release_deferred(inm);
> 		IF_ADDR_RUNLOCK(ifp);
> 	} else {
>@@ -1413,8 +1423,15 @@
> 	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);
>+
>+	if (inm->in6m_refcount == 1) {
>+		/*
>+		 * When there is only one reference left, the "inm"
>+		 * cannot be on any membership lists. It is therefore
>+		 * safe to pass NULL to the "inmh" freelist pointer.
>+		 */
>+		in6m_disconnect_locked(NULL, inm);
>+	}
> 	in6m_release_deferred(inm);
> 	if (ifp)
> 		IF_ADDR_WUNLOCK(ifp);
>Index: sys/netinet6/in6_var.h
>===================================================================
>--- sys/netinet6/in6_var.h	(revision 342455)
>+++ sys/netinet6/in6_var.h	(working copy)
>@@ -670,8 +670,9 @@
> 	}			in6m_st[2];	/* state at t0, t1 */
> };
> 
>-void in6m_disconnect(struct in6_multi *inm);
>+void in6m_disconnect_locked(struct in6_multi_head *inmh, struct in6_multi *inm);
> extern int ifma6_restart;
>+
> /*
>  * Helper function to derive the filter mode on a source entry
>  * from its internal counters. Predicates are:
>@@ -810,6 +811,7 @@
> int	in6m_record_source(struct in6_multi *, const struct in6_addr *);
> void	in6m_release_deferred(struct in6_multi *);
> void	in6m_release_list_deferred(struct in6_multi_head *);
>+void	in6m_release_wait(void);
> void	ip6_freemoptions(struct ip6_moptions *);
> int	ip6_getmoptions(struct inpcb *, struct sockopt *);
> int	ip6_setmoptions(struct inpcb *, struct sockopt *);
>Index: sys/netinet6/mld6.c
>===================================================================
>--- sys/netinet6/mld6.c	(revision 342455)
>+++ sys/netinet6/mld6.c	(working copy)
>@@ -557,7 +557,7 @@
> 				continue;
> 			inm = (struct in6_multi *)ifma->ifma_protospec;
> 			if (inm->in6m_state == MLD_LEAVING_MEMBER) {
>-				in6m_disconnect(inm);
>+				in6m_disconnect_locked(&inmh, inm);
> 				in6m_rele_locked(&inmh, inm);
> 				ifma->ifma_protospec = NULL;
> 			}
>@@ -1484,8 +1484,7 @@
> 	case MLD_REPORTING_MEMBER:
> 		if (report_timer_expired) {
> 			inm->in6m_state = MLD_IDLE_MEMBER;
>-			in6m_disconnect(inm);
>-			in6m_rele_locked(inmh, inm);
>+			in6m_disconnect_locked(inmh, inm);
> 		}
> 		break;
> 	case MLD_G_QUERY_PENDING_MEMBER:
>@@ -1609,7 +1608,7 @@
> 			if (inm->in6m_state == MLD_LEAVING_MEMBER &&
> 			    inm->in6m_scrv == 0) {
> 				inm->in6m_state = MLD_NOT_MEMBER;
>-				in6m_disconnect(inm);
>+				in6m_disconnect_locked(inmh, inm);
> 				in6m_rele_locked(inmh, inm);
> 			}
> 		}
>@@ -1700,7 +1699,7 @@
> 			 * version, we need to release the final
> 			 * reference held for issuing the INCLUDE {}.
> 			 */
>-			in6m_disconnect(inm);
>+			in6m_disconnect_locked(&inmh, inm);
> 			in6m_rele_locked(&inmh, inm);
> 			ifma->ifma_protospec = NULL;
> 			/* FALLTHROUGH */
>@@ -1895,6 +1894,14 @@
> 	error = 0;
> 
> 	/*
>+	 * Check if the in6_multi has already been disconnected.
>+	 */
>+	if (inm->in6m_ifp == NULL) {
>+	  	CTR1(KTR_MLD, "%s: inm is disconnected", __func__);
>+		return (0);
>+	}
>+
>+	/*
> 	 * Try to detect if the upper layer just asked us to change state
> 	 * for an interface which has now gone away.
> 	 */
>@@ -2004,6 +2011,7 @@
> 		if (mli->mli_version == MLD_VERSION_2 &&
> 		    inm->in6m_state == MLD_LEAVING_MEMBER) {
> 			inm->in6m_refcount--;
>+			MPASS(inm->in6m_refcount > 0);
> 		}
> 		inm->in6m_state = MLD_REPORTING_MEMBER;
>

Actions: View | Diff

Attachments on bug 233535: 200199 | 200658 | 200699 | 200753 | 200887 | 200956 | 201125 | 201210 | 201211 | 201212 | 201220 | 201221 | 201228