nwfilter: fix lock order deadlock

Below is backtraces of two deadlocked threads:

thread #1:
 virDomainConfVMNWFilterTeardown
   virNWFilterTeardownFilter
       lock updateMutex <------------
       _virNWFilterTeardownFilter
            try to lock interface <----------

thread #2:
 learnIPAddressThread
    lock interface <-------
    virNWFilterInstantiateFilterLate
        try to lock updateMutex <----------

The problem is fixed by unlocking interface before calling
virNWFilterInstantiateFilterLate to avoid updateMutex and interface ordering
deadlocks. Otherwise we are going to instantiate the filter while holding
interface lock, which will try to lock updateMutex, and if some other thread
instantiating a filter in parallel is holding updateMutex and is trying to
lock interface, both will deadlock.
Also it is safe to unlock interface before virNWFilterInstantiateFilterLate
because learnIPAddressThread stopped capturing packets and applied necessary
rules on the interface, while instantiating a new filter doesn't require a
locked interface.

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>

diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 1adbadbbf888aa650f0811396c68885608594d60..cfd92d951286a21f315fe629c68ce04d319b07fb 100644 (file)
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -611,6 +611,16 @@ learnIPAddressThread(void *arg)
         sa.data.inet4.sin_addr.s_addr = vmaddr;
         char *inetaddr;
 
+        /* It is necessary to unlock interface here to avoid updateMutex and
+         * interface ordering deadlocks. Otherwise we are going to
+         * instantiate the filter, which will try to lock updateMutex, and
+         * some other thread instantiating a filter in parallel is holding
+         * updateMutex and is trying to lock interface, both will deadlock.
+         * Also it is safe to unlock interface here because we stopped
+         * capturing and applied necessary rules on the interface, while
+         * instantiating a new filter doesn't require a locked interface.*/
+        virNWFilterUnlockIface(req->ifname);
+
         if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) {
             if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
                 VIR_ERROR(_("Failed to add IP address %s to IP address "
@@ -636,11 +646,11 @@ learnIPAddressThread(void *arg)
                                  req->ifname, req->ifindex);
 
         techdriver->applyDropAllRules(req->ifname);
+        virNWFilterUnlockIface(req->ifname);
     }
 
     VIR_DEBUG("pcap thread terminating for interface %s", req->ifname);
 
-    virNWFilterUnlockIface(req->ifname);
 
  err_no_lock:
     virNWFilterDeregisterLearnReq(req->ifindex);