svn commit: r325311 - stable/11/sys/netinet

[Alexander Motin]

Author: mav
Date: Thu Nov  2 10:38:09 2017
New Revision: 325311
URL: https://svnweb.freebsd.org/changeset/base/325311

Log:
  MFC r324752: Relax per-ifnet cif_vrs list double locking in carp(4).
  
  In all cases where cif_vrs list is modified, two locks are held: per-ifnet
  CIF_LOCK and global carp_sx.  It means to read that list only one of them
  is enough to be held, so we can skip CIF_LOCK when we already have carp_sx.
  
  This fixes kernel panic, caused by attempts of copyout() to sleep while
  holding non-sleepable CIF_LOCK mutex.

Modified:
  stable/11/sys/netinet/ip_carp.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netinet/ip_carp.c
==============================================================================
--- stable/11/sys/netinet/ip_carp.c	Thu Nov  2 08:47:03 2017	(r325310)
+++ stable/11/sys/netinet/ip_carp.c	Thu Nov  2 10:38:09 2017	(r325311)
@@ -175,8 +175,8 @@ static int proto_reg[] = {-1, -1};
  * Each softc has a lock sc_mtx. It is used to synchronise carp_input_c(),
  * callout-driven events and ioctl()s.
  *
- * To traverse the list of softcs on an ifnet we use CIF_LOCK(), to
- * traverse the global list we use the mutex carp_mtx.
+ * To traverse the list of softcs on an ifnet we use CIF_LOCK() or carp_sx.
+ * To traverse the global list we use the mutex carp_mtx.
  *
  * Known issues with locking:
  *
@@ -286,7 +286,8 @@ SYSCTL_VNET_PCPUSTAT(_net_inet_carp, OID_AUTO, stats, 
 		++_i)
 
 #define	IFNET_FOREACH_CARP(ifp, sc)					\
-	CIF_LOCK_ASSERT(ifp->if_carp);					\
+	KASSERT(mtx_owned(&ifp->if_carp->cif_mtx) ||			\
+	    sx_xlocked(&carp_sx), ("cif_vrs not locked"));		\
 	TAILQ_FOREACH((sc), &(ifp)->if_carp->cif_vrs, sc_list)
 
 #define	DEMOTE_ADVSKEW(sc)					\
@@ -1468,6 +1469,8 @@ carp_alloc(struct ifnet *ifp)
 	struct carp_softc *sc;
 	struct carp_if *cif;
 
+	sx_assert(&carp_sx, SA_XLOCKED);
+
 	if ((cif = ifp->if_carp) == NULL)
 		cif = carp_alloc_if(ifp);
 
@@ -1657,11 +1660,9 @@ carp_ioctl(struct ifreq *ifr, u_long cmd, struct threa
 		}
 
 		if (ifp->if_carp) {
-			CIF_LOCK(ifp->if_carp);
 			IFNET_FOREACH_CARP(ifp, sc)
 				if (sc->sc_vhid == carpr.carpr_vhid)
 					break;
-			CIF_UNLOCK(ifp->if_carp);
 		}
 		if (sc == NULL) {
 			sc = carp_alloc(ifp);
@@ -1732,11 +1733,9 @@ carp_ioctl(struct ifreq *ifr, u_long cmd, struct threa
 
 		priveleged = (priv_check(td, PRIV_NETINET_CARP) == 0);
 		if (carpr.carpr_vhid != 0) {
-			CIF_LOCK(ifp->if_carp);
 			IFNET_FOREACH_CARP(ifp, sc)
 				if (sc->sc_vhid == carpr.carpr_vhid)
 					break;
-			CIF_UNLOCK(ifp->if_carp);
 			if (sc == NULL) {
 				error = ENOENT;
 				break;
@@ -1747,7 +1746,6 @@ carp_ioctl(struct ifreq *ifr, u_long cmd, struct threa
 			int i, count;
 
 			count = 0;
-			CIF_LOCK(ifp->if_carp);
 			IFNET_FOREACH_CARP(ifp, sc)
 				count++;
 
@@ -1769,7 +1767,6 @@ carp_ioctl(struct ifreq *ifr, u_long cmd, struct threa
 				}
 				i++;
 			}
-			CIF_UNLOCK(ifp->if_carp);
 		}
 		break;
 	    }
@@ -1824,11 +1821,9 @@ carp_attach(struct ifaddr *ifa, int vhid)
 		return (ENOPROTOOPT);
 	}
 
-	CIF_LOCK(cif);
 	IFNET_FOREACH_CARP(ifp, sc)
 		if (sc->sc_vhid == vhid)
 			break;
-	CIF_UNLOCK(cif);
 	if (sc == NULL) {
 		sx_xunlock(&carp_sx);
 		return (ENOENT);