In the not so distant past, the lock ordering in virNWFilterLockIface() was as follows: global mutex ifaceMapLock was acquired, then internal representation of given interface was looked up in a hash table (or created brand new if none was found), the global lock was released and the lock of the interface was acquired. But this was mistakenly changed as the function was rewritten to use automatic mutexes, because now the global lock is held throughout the whole run of the function and thus the interface specific lock is acquired with the global lock held. This results in a deadlock. Fixes: dd8150c48dcf94e8d3b0481be08eeef822b98b02 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/nwfilter/nwfilter_learnipaddr.c | 49 +++++++++++++++-------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 2c85972012..ec2d337188 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -143,37 +143,40 @@ static bool threadsTerminate; int virNWFilterLockIface(const char *ifname) { - VIR_LOCK_GUARD lock = virLockGuardLock(&ifaceMapLock); - virNWFilterIfaceLock *ifaceLock = virHashLookup(ifaceLockMap, ifname); + virNWFilterIfaceLock *ifaceLock = NULL; - if (!ifaceLock) { - ifaceLock = g_new0(virNWFilterIfaceLock, 1); + VIR_WITH_MUTEX_LOCK_GUARD(&ifaceMapLock) { + ifaceLock = virHashLookup(ifaceLockMap, ifname); - if (virMutexInitRecursive(&ifaceLock->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("mutex initialization failed")); - g_free(ifaceLock); - return -1; - } + if (!ifaceLock) { + ifaceLock = g_new0(virNWFilterIfaceLock, 1); - if (virStrcpyStatic(ifaceLock->ifname, ifname) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("interface name %s does not fit into buffer"), - ifaceLock->ifname); - g_free(ifaceLock); - return -1; - } + if (virMutexInitRecursive(&ifaceLock->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("mutex initialization failed")); + g_free(ifaceLock); + return -1; + } + + if (virStrcpyStatic(ifaceLock->ifname, ifname) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("interface name %s does not fit into buffer"), + ifaceLock->ifname); + g_free(ifaceLock); + return -1; + } + + while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) { + g_free(ifaceLock); + return -1; + } - while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) { - g_free(ifaceLock); - return -1; + ifaceLock->refctr = 0; } - ifaceLock->refctr = 0; + ifaceLock->refctr++; } - ifaceLock->refctr++; - virMutexLock(&ifaceLock->lock); return 0; -- 2.34.1