On Fri, Apr 22, 2022 at 04:45:37PM +0200, Michal Privoznik wrote:
When one thread is trying to reload NWFilter driver (by running
nwfilterStateReload()) but there's another thread that's
concurrently running nwfilterStateCleanup() a crash may occur.
This is despite nwfilterStateReload() checking for driver !=
NULL, because is done so without @driverMutex held. A typical
TOCTOU. Fortunately, the mutex is always initialized, so the
mutex can be acquired at all times and @driver can be checked
with the lock held.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=2075837
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/nwfilter/nwfilter_driver.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
I'm concerned that we might have latent bugs in other
drivers too, and/or be at risk of introducing them
later.
I've always thought of StateReload as been non-overlapping
with StateCleanup, though I realize now that's not actually
the case in practice.
I wonder if it would better if we made remote_daemon.c
avoiding calling StateCleanup, until any StateReload
has finished, to eliminate the entire class of problem
across all the drivers.
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index b66ba22737..d028efafbe 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -309,6 +309,8 @@ nwfilterStateInitialize(bool privileged,
static int
nwfilterStateReload(void)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driverMutex);
+
if (!driver)
return -1;
@@ -319,15 +321,13 @@ nwfilterStateReload(void)
/* shut down all threads -- they will be restarted if necessary */
virNWFilterLearnThreadsTerminate(true);
- VIR_WITH_MUTEX_LOCK_GUARD(&driverMutex) {
- VIR_WITH_MUTEX_LOCK_GUARD(&driver->updateLock) {
- virNWFilterObjListLoadAllConfigs(driver->nwfilters,
driver->configDir);
- }
-
-
- virNWFilterBuildAll(driver, false);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->updateLock) {
+ virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir);
}
+
+ virNWFilterBuildAll(driver, false);
+
return 0;
}
--
2.35.1
With regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|