Author: mav
Date: Thu Oct 19 09:01:15 2017
New Revision: 324752
URL: https://svnweb.freebsd.org/changeset/base/324752

Log:
  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.
  
  Discussed with:       glebius
  MFC after:    2 weeks
  Sponsored by: iXsystems, Inc.

Modified:
  head/sys/netinet/ip_carp.c

Modified: head/sys/netinet/ip_carp.c
==============================================================================
--- head/sys/netinet/ip_carp.c  Thu Oct 19 08:17:32 2017        (r324751)
+++ head/sys/netinet/ip_carp.c  Thu Oct 19 09:01:15 2017        (r324752)
@@ -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)                                      \
@@ -1562,6 +1563,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);
 
@@ -1751,11 +1754,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);
@@ -1826,11 +1827,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;
@@ -1841,7 +1840,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++;
 
@@ -1863,7 +1861,6 @@ carp_ioctl(struct ifreq *ifr, u_long cmd, struct threa
                                }
                                i++;
                        }
-                       CIF_UNLOCK(ifp->if_carp);
                }
                break;
            }
@@ -1918,11 +1915,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);
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to