d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Currently in_pcbfree will unconditionally wunlock the pcbinfo lock

Browse Source 
to avoid a LOR on the multicast list lock in the freemoptions routines.
As it turns out, tcp_usr_detach can acquire the tcbinfo lock readonly.
Trying to wunlock the pcbinfo lock in that context has caused a number
of reported crashes.

This change unclutters in_pcbfree and moves the handling of wunlock vs
runlock of pcbinfo to the freemoptions routine.

Reported by:	mjg@, bde@, o.hartmann at walstatt.org
Approved by:	sbruno
This commit is contained in:
Matt Macy 2018-05-05 22:40:40 +00:00
parent b6f2c452cb
commit 28c001002a
5 changed files with 32 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
sys/netinet/in_mcast.c
View File

@ -1639,16 +1639,29 @@ inp_findmoptions(struct inpcb *inp)
* SMPng: NOTE: assumes INP write lock is held. * SMPng: NOTE: assumes INP write lock is held.
*/ */
void void
inp_freemoptions(struct ip_moptions *imo) inp_freemoptions(struct ip_moptions *imo, struct inpcbinfo *pcbinfo)
{ {
int wlock;
if (imo == NULL) if (imo == NULL)
return; return;
INP_INFO_LOCK_ASSERT(pcbinfo);
wlock = INP_INFO_WLOCKED(pcbinfo);
if (wlock)
INP_INFO_WUNLOCK(pcbinfo);
else
INP_INFO_RUNLOCK(pcbinfo);
KASSERT(imo != NULL, ("%s: ip_moptions is NULL", __func__)); KASSERT(imo != NULL, ("%s: ip_moptions is NULL", __func__));
IN_MULTI_LIST_LOCK(); IN_MULTI_LIST_LOCK();
STAILQ_INSERT_TAIL(&imo_gc_list, imo, imo_link); STAILQ_INSERT_TAIL(&imo_gc_list, imo, imo_link);
IN_MULTI_LIST_UNLOCK(); IN_MULTI_LIST_UNLOCK();
taskqueue_enqueue(taskqueue_thread, &imo_gc_task); taskqueue_enqueue(taskqueue_thread, &imo_gc_task);
if (wlock)
INP_INFO_WLOCK(pcbinfo);
else
INP_INFO_RLOCK(pcbinfo);
} }
static void static void

6
sys/netinet/in_pcb.c
View File

@ -1388,14 +1388,12 @@ in_pcbfree(struct inpcb *inp)
if (imo == NULL && im6o == NULL) if (imo == NULL && im6o == NULL)
return; return;
#endif #endif
INP_INFO_WUNLOCK(pcbinfo);
#ifdef INET6 #ifdef INET6
ip6_freemoptions(im6o); ip6_freemoptions(im6o, pcbinfo);
#endif #endif
#ifdef INET #ifdef INET
inp_freemoptions(imo); inp_freemoptions(imo, pcbinfo);
#endif #endif
INP_INFO_WLOCK(pcbinfo);
} }
/* /*

3
sys/netinet/ip_var.h
View File

@ -175,6 +175,7 @@ struct ip;
struct inpcb; struct inpcb;
struct route; struct route;
struct sockopt; struct sockopt;
struct inpcbinfo;
VNET_DECLARE(int, ip_defttl); /* default IP ttl */ VNET_DECLARE(int, ip_defttl); /* default IP ttl */
VNET_DECLARE(int, ipforwarding); /* ip forwarding */ VNET_DECLARE(int, ipforwarding); /* ip forwarding */
@ -201,7 +202,7 @@ extern struct pr_usrreqs rip_usrreqs;
#define V_rsvp_on VNET(rsvp_on) #define V_rsvp_on VNET(rsvp_on)
#define V_drop_redirect VNET(drop_redirect) #define V_drop_redirect VNET(drop_redirect)
void inp_freemoptions(struct ip_moptions *); void inp_freemoptions(struct ip_moptions *, struct inpcbinfo *);
int inp_getmoptions(struct inpcb *, struct sockopt *); int inp_getmoptions(struct inpcb *, struct sockopt *);
int inp_setmoptions(struct inpcb *, struct sockopt *); int inp_setmoptions(struct inpcb *, struct sockopt *);

13
sys/netinet6/in6_mcast.c
View File

@ -1585,13 +1585,20 @@ in6p_findmoptions(struct inpcb *inp)
* SMPng: NOTE: assumes INP write lock is held. * SMPng: NOTE: assumes INP write lock is held.
*/ */
void void
ip6_freemoptions(struct ip6_moptions *imo) ip6_freemoptions(struct ip6_moptions *imo, struct inpcbinfo *pcbinfo)
{ {
struct in6_mfilter *imf; struct in6_mfilter *imf;
size_t idx, nmships; size_t idx, nmships;
int wlock;
if (imo == NULL) if (imo == NULL)
return; return;
INP_INFO_LOCK_ASSERT(pcbinfo);
wlock = INP_INFO_WLOCKED(pcbinfo);
if (wlock)
INP_INFO_WUNLOCK(pcbinfo);
else
INP_INFO_RUNLOCK(pcbinfo);
nmships = imo->im6o_num_memberships; nmships = imo->im6o_num_memberships;
for (idx = 0; idx < nmships; ++idx) { for (idx = 0; idx < nmships; ++idx) {
@ -1608,6 +1615,10 @@ ip6_freemoptions(struct ip6_moptions *imo)
free(imo->im6o_mfilters, M_IN6MFILTER); free(imo->im6o_mfilters, M_IN6MFILTER);
free(imo->im6o_membership, M_IP6MOPTS); free(imo->im6o_membership, M_IP6MOPTS);
free(imo, M_IP6MOPTS); free(imo, M_IP6MOPTS);
if (wlock)
INP_INFO_WLOCK(pcbinfo);
else
INP_INFO_RLOCK(pcbinfo);
} }
/* /*

3
sys/netinet6/in6_var.h
View File

@ -807,6 +807,7 @@ in6m_rele_locked(struct in6_multi_head *inmh, struct in6_multi *inm)
struct ip6_moptions; struct ip6_moptions;
struct sockopt; struct sockopt;
struct inpcbinfo;
/* Multicast KPIs. */ /* Multicast KPIs. */
int im6o_mc_filter(const struct ip6_moptions *, const struct ifnet *, int im6o_mc_filter(const struct ip6_moptions *, const struct ifnet *,
@ -823,7 +824,7 @@ void in6m_print(const struct in6_multi *);
int in6m_record_source(struct in6_multi *, const struct in6_addr *); int in6m_record_source(struct in6_multi *, const struct in6_addr *);
void in6m_release_deferred(struct in6_multi *); void in6m_release_deferred(struct in6_multi *);
void in6m_release_list_deferred(struct in6_multi_head *); void in6m_release_list_deferred(struct in6_multi_head *);
void ip6_freemoptions(struct ip6_moptions *); void ip6_freemoptions(struct ip6_moptions *, struct inpcbinfo *);
int ip6_getmoptions(struct inpcb *, struct sockopt *); int ip6_getmoptions(struct inpcb *, struct sockopt *);
int ip6_setmoptions(struct inpcb *, struct sockopt *); int ip6_setmoptions(struct inpcb *, struct sockopt *);