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>