Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 16 May 2019 13:04:26 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r347691 - in head/sys: netinet netinet6
Message-ID:  <201905161304.x4GD4QYU061147@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: markj
Date: Thu May 16 13:04:26 2019
New Revision: 347691
URL: https://svnweb.freebsd.org/changeset/base/347691

Log:
  Revert r347582 for now.
  
  The inp lock still needs to be dropped when calling into the driver ioctl
  handler, as some drivers expect to be able to sleep.
  
  Reported by:	kib

Modified:
  head/sys/netinet/in_mcast.c
  head/sys/netinet6/in6_mcast.c

Modified: head/sys/netinet/in_mcast.c
==============================================================================
--- head/sys/netinet/in_mcast.c	Thu May 16 13:03:54 2019	(r347690)
+++ head/sys/netinet/in_mcast.c	Thu May 16 13:04:26 2019	(r347691)
@@ -1534,7 +1534,6 @@ inp_block_unblock_source(struct inpcb *inp, struct soc
 	/*
 	 * Check if we are actually a member of this group.
 	 */
-	IN_MULTI_LOCK();
 	imo = inp_findmoptions(inp);
 	idx = imo_match_group(imo, ifp, &gsa->sa);
 	if (idx == -1 || imo->imo_mfilters == NULL) {
@@ -1594,13 +1593,14 @@ inp_block_unblock_source(struct inpcb *inp, struct soc
 	/*
 	 * Begin state merge transaction at IGMP layer.
 	 */
+	IN_MULTI_LOCK();
 	CTR1(KTR_IGMPV3, "%s: merge inm state", __func__);
 	IN_MULTI_LIST_LOCK();
 	error = inm_merge(inm, imf);
 	if (error) {
 		CTR1(KTR_IGMPV3, "%s: failed to merge inm state", __func__);
 		IN_MULTI_LIST_UNLOCK();
-		goto out_imf_rollback;
+		goto out_in_multi_locked;
 	}
 
 	CTR1(KTR_IGMPV3, "%s: doing igmp downcall", __func__);
@@ -1609,6 +1609,9 @@ inp_block_unblock_source(struct inpcb *inp, struct soc
 	if (error)
 		CTR1(KTR_IGMPV3, "%s: failed igmp downcall", __func__);
 
+out_in_multi_locked:
+
+	IN_MULTI_UNLOCK();
 out_imf_rollback:
 	if (error)
 		imf_rollback(imf);
@@ -1619,7 +1622,6 @@ out_imf_rollback:
 
 out_inp_locked:
 	INP_WUNLOCK(inp);
-	IN_MULTI_UNLOCK();
 	return (error);
 }
 
@@ -1678,10 +1680,10 @@ inp_findmoptions(struct inpcb *inp)
 static void
 inp_gcmoptions(struct ip_moptions *imo)
 {
-	struct in_mfilter *imf;
+	struct in_mfilter	*imf;
 	struct in_multi *inm;
 	struct ifnet *ifp;
-	size_t idx, nmships;
+	size_t			 idx, nmships;
 
 	nmships = imo->imo_num_memberships;
 	for (idx = 0; idx < nmships; ++idx) {
@@ -2140,12 +2142,12 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt
 		CTR2(KTR_IGMPV3, "%s: unknown sopt_name %d",
 		    __func__, sopt->sopt_name);
 		return (EOPNOTSUPP);
+		break;
 	}
 
 	if (ifp == NULL || (ifp->if_flags & IFF_MULTICAST) == 0)
 		return (EADDRNOTAVAIL);
 
-	IN_MULTI_LOCK();
 	imo = inp_findmoptions(inp);
 	idx = imo_match_group(imo, ifp, &gsa->sa);
 	if (idx == -1) {
@@ -2270,6 +2272,10 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt
 	/*
 	 * Begin state merge transaction at IGMP layer.
 	 */
+	in_pcbref(inp);
+	INP_WUNLOCK(inp);
+	IN_MULTI_LOCK();
+
 	if (is_new) {
 		error = in_joingroup_locked(ifp, &gsa->sin.sin_addr, imf,
 		    &inm);
@@ -2280,8 +2286,6 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt
 			goto out_imo_free;
 		}
 		inm_acquire(inm);
-		KASSERT(imo->imo_membership[idx] == NULL,
-		    ("%s: imo_membership already allocated", __func__));
 		imo->imo_membership[idx] = inm;
 	} else {
 		CTR1(KTR_IGMPV3, "%s: merge inm state", __func__);
@@ -2291,7 +2295,7 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt
 			CTR1(KTR_IGMPV3, "%s: failed to merge inm state",
 				 __func__);
 			IN_MULTI_LIST_UNLOCK();
-			goto out_imf_rollback;
+			goto out_in_multi_locked;
 		}
 		CTR1(KTR_IGMPV3, "%s: doing igmp downcall", __func__);
 		error = igmp_change_state(inm);
@@ -2299,11 +2303,16 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt
 		if (error) {
 			CTR1(KTR_IGMPV3, "%s: failed igmp downcall",
 			    __func__);
-			goto out_imf_rollback;
+			goto out_in_multi_locked;
 		}
 	}
 
-out_imf_rollback:
+out_in_multi_locked:
+
+	IN_MULTI_UNLOCK();
+	INP_WLOCK(inp);
+	if (in_pcbrele_wlocked(inp))
+		return (ENXIO);
 	if (error) {
 		imf_rollback(imf);
 		if (is_new)
@@ -2328,7 +2337,6 @@ out_imo_free:
 
 out_inp_locked:
 	INP_WUNLOCK(inp);
-	IN_MULTI_UNLOCK();
 	return (error);
 }
 
@@ -2455,7 +2463,6 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sop
 	/*
 	 * Find the membership in the membership array.
 	 */
-	IN_MULTI_LOCK();
 	imo = inp_findmoptions(inp);
 	idx = imo_match_group(imo, ifp, &gsa->sa);
 	if (idx == -1) {
@@ -2503,6 +2510,9 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sop
 	/*
 	 * Begin state merge transaction at IGMP layer.
 	 */
+	in_pcbref(inp);
+	INP_WUNLOCK(inp);
+	IN_MULTI_LOCK();
 
 	if (is_final) {
 		/*
@@ -2518,7 +2528,7 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sop
 			CTR1(KTR_IGMPV3, "%s: failed to merge inm state",
 			    __func__);
 			IN_MULTI_LIST_UNLOCK();
-			goto out_imf_rollback;
+			goto out_in_multi_locked;
 		}
 
 		CTR1(KTR_IGMPV3, "%s: doing igmp downcall", __func__);
@@ -2530,7 +2540,13 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sop
 		}
 	}
 
-out_imf_rollback:
+out_in_multi_locked:
+
+	IN_MULTI_UNLOCK();
+	INP_WLOCK(inp);
+	if (in_pcbrele_wlocked(inp))
+		return (ENXIO);
+
 	if (error)
 		imf_rollback(imf);
 	else
@@ -2541,7 +2557,7 @@ out_imf_rollback:
 	if (is_final) {
 		/* Remove the gap in the membership and filter array. */
 		KASSERT(RB_EMPTY(&imf->imf_sources),
-		    ("%s: imf_sources (%p %p %zu) not empty", __func__, imf, imo, idx));
+		    ("%s: imf_sources not empty", __func__));
 		for (++idx; idx < imo->imo_num_memberships; ++idx) {
 			imo->imo_membership[idx - 1] = imo->imo_membership[idx];
 			imo->imo_mfilters[idx - 1] = imo->imo_mfilters[idx];
@@ -2553,7 +2569,6 @@ out_imf_rollback:
 
 out_inp_locked:
 	INP_WUNLOCK(inp);
-	IN_MULTI_UNLOCK();
 	return (error);
 }
 
@@ -2631,6 +2646,8 @@ inp_set_multicast_if(struct inpcb *inp, struct sockopt
 
 /*
  * Atomically set source filters on a socket for an IPv4 multicast group.
+ *
+ * SMPng: NOTE: Potentially calls malloc(M_WAITOK) with Giant held.
  */
 static int
 inp_set_source_filters(struct inpcb *inp, struct sockopt *sopt)
@@ -2677,7 +2694,6 @@ inp_set_source_filters(struct inpcb *inp, struct socko
 	 * Take the INP write lock.
 	 * Check if this socket is a member of this group.
 	 */
-	IN_MULTI_LOCK();
 	imo = inp_findmoptions(inp);
 	idx = imo_match_group(imo, ifp, &gsa->sa);
 	if (idx == -1 || imo->imo_mfilters == NULL) {
@@ -2762,6 +2778,7 @@ inp_set_source_filters(struct inpcb *inp, struct socko
 		goto out_imf_rollback;
 
 	INP_WLOCK_ASSERT(inp);
+	IN_MULTI_LOCK();
 
 	/*
 	 * Begin state merge transaction at IGMP layer.
@@ -2772,7 +2789,7 @@ inp_set_source_filters(struct inpcb *inp, struct socko
 	if (error) {
 		CTR1(KTR_IGMPV3, "%s: failed to merge inm state", __func__);
 		IN_MULTI_LIST_UNLOCK();
-		goto out_imf_rollback;
+		goto out_in_multi_locked;
 	}
 
 	CTR1(KTR_IGMPV3, "%s: doing igmp downcall", __func__);
@@ -2781,6 +2798,10 @@ inp_set_source_filters(struct inpcb *inp, struct socko
 	if (error)
 		CTR1(KTR_IGMPV3, "%s: failed igmp downcall", __func__);
 
+out_in_multi_locked:
+
+	IN_MULTI_UNLOCK();
+
 out_imf_rollback:
 	if (error)
 		imf_rollback(imf);
@@ -2791,7 +2812,6 @@ out_imf_rollback:
 
 out_inp_locked:
 	INP_WUNLOCK(inp);
-	IN_MULTI_UNLOCK();
 	return (error);
 }
 

Modified: head/sys/netinet6/in6_mcast.c
==============================================================================
--- head/sys/netinet6/in6_mcast.c	Thu May 16 13:03:54 2019	(r347690)
+++ head/sys/netinet6/in6_mcast.c	Thu May 16 13:04:26 2019	(r347691)
@@ -2052,7 +2052,6 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sop
 	 */
 	(void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL);
 
-	IN6_MULTI_LOCK();
 	imo = in6p_findmoptions(inp);
 	idx = im6o_match_group(imo, ifp, &gsa->sa);
 	if (idx == -1) {
@@ -2172,6 +2171,10 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sop
 	/*
 	 * Begin state merge transaction at MLD layer.
 	 */
+	in_pcbref(inp);
+	INP_WUNLOCK(inp);
+	IN6_MULTI_LOCK();
+
 	if (is_new) {
 		error = in6_joingroup_locked(ifp, &gsa->sin6.sin6_addr, imf,
 		    &inm, 0);
@@ -2201,6 +2204,10 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sop
 		IN6_MULTI_LIST_UNLOCK();
 	}
 
+	IN6_MULTI_UNLOCK();
+	INP_WLOCK(inp);
+	if (in_pcbrele_wlocked(inp))
+		return (ENXIO);
 	if (error) {
 		im6f_rollback(imf);
 		if (is_new)
@@ -2225,7 +2232,6 @@ out_im6o_free:
 
 out_in6p_locked:
 	INP_WUNLOCK(inp);
-	IN6_MULTI_UNLOCK();
 	in6m_release_list_deferred(&inmh);
 	return (error);
 }
@@ -2375,7 +2381,6 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *so
 	/*
 	 * Find the membership in the membership array.
 	 */
-	IN6_MULTI_LOCK();
 	imo = in6p_findmoptions(inp);
 	idx = im6o_match_group(imo, ifp, &gsa->sa);
 	if (idx == -1) {
@@ -2424,6 +2429,10 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *so
 	/*
 	 * Begin state merge transaction at MLD layer.
 	 */
+	in_pcbref(inp);
+	INP_WUNLOCK(inp);
+	IN6_MULTI_LOCK();
+
 	if (is_final) {
 		/*
 		 * Give up the multicast address record to which
@@ -2447,6 +2456,11 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *so
 		IN6_MULTI_LIST_UNLOCK();
 	}
 
+	IN6_MULTI_UNLOCK();
+	INP_WLOCK(inp);
+	if (in_pcbrele_wlocked(inp))
+		return (ENXIO);
+
 	if (error)
 		im6f_rollback(imf);
 	else
@@ -2469,7 +2483,6 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *so
 
 out_in6p_locked:
 	INP_WUNLOCK(inp);
-	IN6_MULTI_UNLOCK();
 	return (error);
 }
 
@@ -2515,6 +2528,8 @@ in6p_set_multicast_if(struct inpcb *inp, struct sockop
 
 /*
  * Atomically set source filters on a socket for an IPv6 multicast group.
+ *
+ * SMPng: NOTE: Potentially calls malloc(M_WAITOK) with Giant held.
  */
 static int
 in6p_set_source_filters(struct inpcb *inp, struct sockopt *sopt)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905161304.x4GD4QYU061147>