From 879ecd602fef31f5ce78beda8562fe2974e65241 Mon Sep 17 00:00:00 2001 From: Tim Wiederhake Date: Fri, 8 Apr 2022 12:50:35 +0200 Subject: [PATCH] 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 Reviewed-by: Michal Privoznik --- src/nwfilter/nwfilter_dhcpsnoop.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 852840c209..d26e787453 100644 --- 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 */