virNWFilterSnoopState: Prevent mutex leak

virNWFilterDHCPSnoopShutdown would never destroy the mutexes created
in virNWFilterDHCPSnoopInit. Additionally, if in virNWFilterDHCPSnoopInit
the call to virMutexInitRecursive succeeds and the call to virMutexInit
fails, this would lead to either virNWFilterSnoopState.snoopLock being
initialized twice or virNWFilterSnoopState.activeLock destroyed without
being initialized first.

This enables a later patch to use virNWFilterDHCPSnoopShutdown as a
cleanup function safely, as it is a no-op if virNWFilterSnoopState was
not yet initialized.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index 852840c20931f9fd5b64a0cc708a22701755caba..d26e787453697f6c2c44745ab3ad4f5baed143e5 100644 (file)
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1860,10 +1860,14 @@ virNWFilterDHCPSnoopInit(void)
 
     VIR_DEBUG("Initializing DHCP snooping");
 
-    if (virMutexInitRecursive(&virNWFilterSnoopState.snoopLock) < 0 ||
-        virMutexInit(&virNWFilterSnoopState.activeLock) < 0)
+    if (virMutexInitRecursive(&virNWFilterSnoopState.snoopLock) < 0)
         return -1;
 
+    if (virMutexInit(&virNWFilterSnoopState.activeLock) < 0) {
+        virMutexDestroy(&virNWFilterSnoopState.snoopLock);
+        return -1;
+    }
+
     virNWFilterSnoopState.ifnameToKey = virHashNew(NULL);
     virNWFilterSnoopState.active = virHashNew(NULL);
     virNWFilterSnoopState.snoopReqs =
@@ -1938,6 +1942,9 @@ virNWFilterDHCPSnoopEnd(const char *ifname)
 void
 virNWFilterDHCPSnoopShutdown(void)
 {
+    if (!virNWFilterSnoopState.snoopReqs)
+        return;
+
     virNWFilterSnoopEndThreads();
     virNWFilterSnoopJoinThreads();
 
@@ -1947,9 +1954,13 @@ virNWFilterDHCPSnoopShutdown(void)
         g_clear_pointer(&virNWFilterSnoopState.snoopReqs, g_hash_table_unref);
     }
 
+    virMutexDestroy(&virNWFilterSnoopState.snoopLock);
+
     VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.activeLock) {
         g_clear_pointer(&virNWFilterSnoopState.active, g_hash_table_unref);
     }
+
+    virMutexDestroy(&virNWFilterSnoopState.activeLock);
 }
 
 #else /* WITH_LIBPCAP */