[libvirt PATCH 03/10] secret: Factor out mutex

Tim Wiederhake posted 10 patches 3 years, 12 months ago
[libvirt PATCH 03/10] secret: Factor out mutex
Posted by Tim Wiederhake 3 years, 12 months ago
If the mutex is part of the `driver` object, it cannot guard that
object's creation and destruction perfectly.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 src/secret/secret_driver.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 0220f394ef..d0e819809b 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -52,9 +52,10 @@ enum { SECRET_MAX_XML_FILE = 10*1024*1024 };
 
 /* Internal driver state */
 
+static virMutex mutex = VIR_MUTEX_INITIALIZER;
+
 typedef struct _virSecretDriverState virSecretDriverState;
 struct _virSecretDriverState {
-    virMutex lock;
     bool privileged; /* readonly */
     char *embeddedRoot; /* readonly */
     int embeddedRefs;
@@ -74,14 +75,14 @@ static virSecretDriverState *driver;
 static void
 secretDriverLock(void)
 {
-    virMutexLock(&driver->lock);
+    virMutexLock(&mutex);
 }
 
 
 static void
 secretDriverUnlock(void)
 {
-    virMutexUnlock(&driver->lock);
+    virMutexUnlock(&mutex);
 }
 
 
@@ -463,7 +464,6 @@ secretStateCleanup(void)
 
     VIR_FREE(driver->stateDir);
     secretDriverUnlock();
-    virMutexDestroy(&driver->lock);
     VIR_FREE(driver);
 
     return 0;
@@ -479,10 +479,6 @@ secretStateInitialize(bool privileged,
     driver = g_new0(virSecretDriverState, 1);
 
     driver->lockFD = -1;
-    if (virMutexInit(&driver->lock) < 0) {
-        VIR_FREE(driver);
-        return VIR_DRV_STATE_INIT_ERROR;
-    }
     secretDriverLock();
 
     driver->secretEventState = virObjectEventStateNew();
-- 
2.31.1

Re: [libvirt PATCH 03/10] secret: Factor out mutex
Posted by Daniel P. Berrangé 3 years, 11 months ago
On Fri, Feb 11, 2022 at 11:30:39AM +0100, Tim Wiederhake wrote:
> If the mutex is part of the `driver` object, it cannot guard that
> object's creation and destruction perfectly.

The mutex doesn't need to guard the object's creation/destruction
in its entirity though.

The driver creation/destruction is a onetime thing at startup
and shutdown of the daemon. There is a requirement that API
calls have ceased before destruction begins, and if that's not
the case then the code is unsafe no matter what because it
will be liable to access a NULL driver object after acquiring
the mutex.

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 :|