Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 5 May 2018 22:40:40 +0000 (UTC)
From:      Matt Macy <mmacy@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r333277 - in head/sys: netinet netinet6
Message-ID:  <201805052240.w45MeeKM020551@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mmacy
Date: Sat May  5 22:40:40 2018
New Revision: 333277
URL: https://svnweb.freebsd.org/changeset/base/333277

Log:
  Currently in_pcbfree will unconditionally wunlock the pcbinfo lock
  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

Modified:
  head/sys/netinet/in_mcast.c
  head/sys/netinet/in_pcb.c
  head/sys/netinet/ip_var.h
  head/sys/netinet6/in6_mcast.c
  head/sys/netinet6/in6_var.h

Modified: head/sys/netinet/in_mcast.c
==============================================================================
--- head/sys/netinet/in_mcast.c	Sat May  5 20:16:08 2018	(r333276)
+++ head/sys/netinet/in_mcast.c	Sat May  5 22:40:40 2018	(r333277)
@@ -1639,16 +1639,29 @@ inp_findmoptions(struct inpcb *inp)
  * SMPng: NOTE: assumes INP write lock is held.
  */
 void
-inp_freemoptions(struct ip_moptions *imo)
+inp_freemoptions(struct ip_moptions *imo, struct inpcbinfo *pcbinfo)
 {
+	int wlock;
 
 	if (imo == NULL)
 		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__));
 	IN_MULTI_LIST_LOCK();
 	STAILQ_INSERT_TAIL(&imo_gc_list, imo, imo_link);
 	IN_MULTI_LIST_UNLOCK();
 	taskqueue_enqueue(taskqueue_thread, &imo_gc_task);
+	if (wlock)
+		INP_INFO_WLOCK(pcbinfo);
+	else
+		INP_INFO_RLOCK(pcbinfo);
 }
 
 static void

Modified: head/sys/netinet/in_pcb.c
==============================================================================
--- head/sys/netinet/in_pcb.c	Sat May  5 20:16:08 2018	(r333276)
+++ head/sys/netinet/in_pcb.c	Sat May  5 22:40:40 2018	(r333277)
@@ -1388,14 +1388,12 @@ in_pcbfree(struct inpcb *inp)
 	if (imo == NULL && im6o == NULL)
 		return;
 #endif
-	INP_INFO_WUNLOCK(pcbinfo);
 #ifdef INET6
-	ip6_freemoptions(im6o);
+	ip6_freemoptions(im6o, pcbinfo);
 #endif
 #ifdef INET
-	inp_freemoptions(imo);
+	inp_freemoptions(imo, pcbinfo);
 #endif
-	INP_INFO_WLOCK(pcbinfo);
 }
 
 /*

Modified: head/sys/netinet/ip_var.h
==============================================================================
--- head/sys/netinet/ip_var.h	Sat May  5 20:16:08 2018	(r333276)
+++ head/sys/netinet/ip_var.h	Sat May  5 22:40:40 2018	(r333277)
@@ -175,6 +175,7 @@ struct ip;
 struct inpcb;
 struct route;
 struct sockopt;
+struct inpcbinfo;
 
 VNET_DECLARE(int, ip_defttl);			/* default IP ttl */
 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_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_setmoptions(struct inpcb *, struct sockopt *);
 

Modified: head/sys/netinet6/in6_mcast.c
==============================================================================
--- head/sys/netinet6/in6_mcast.c	Sat May  5 20:16:08 2018	(r333276)
+++ head/sys/netinet6/in6_mcast.c	Sat May  5 22:40:40 2018	(r333277)
@@ -1585,13 +1585,20 @@ in6p_findmoptions(struct inpcb *inp)
  * SMPng: NOTE: assumes INP write lock is held.
  */
 void
-ip6_freemoptions(struct ip6_moptions *imo)
+ip6_freemoptions(struct ip6_moptions *imo, struct inpcbinfo *pcbinfo)
 {
 	struct in6_mfilter	*imf;
 	size_t			 idx, nmships;
+	int wlock;
 
 	if (imo == NULL)
 		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;
 	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_membership, M_IP6MOPTS);
 	free(imo, M_IP6MOPTS);
+	if (wlock)
+		INP_INFO_WLOCK(pcbinfo);
+	else
+		INP_INFO_RLOCK(pcbinfo);
 }
 
 /*

Modified: head/sys/netinet6/in6_var.h
==============================================================================
--- head/sys/netinet6/in6_var.h	Sat May  5 20:16:08 2018	(r333276)
+++ head/sys/netinet6/in6_var.h	Sat May  5 22:40:40 2018	(r333277)
@@ -807,6 +807,7 @@ in6m_rele_locked(struct in6_multi_head *inmh, struct i
 
 struct ip6_moptions;
 struct sockopt;
+struct inpcbinfo;
 
 /* Multicast KPIs. */
 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 *);
 void	in6m_release_deferred(struct in6_multi *);
 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_setmoptions(struct inpcb *, struct sockopt *);
 



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