[libvirt] [PATCH v4 16/23] security_manager: Introduce metadata locking APIs

Michal Privoznik mprivozn at redhat.com
Mon Sep 10 09:36:17 UTC 2018


Two new APIs are added so that security driver can lock and
unlock paths it wishes to touch. These APIs are not for other
drivers to call but security drivers (DAC and SELinux). That is
the reason these APIs are not exposed through our
libvirt_private.syms file.

Three interesting things happen in this commit. The first is the
global @lockManagerMutex. Unfortunately, this has to exist so that
there is only on thread talking to virtlockd at a time. If there
were more threads and one of them closed the connection
prematurely, it would cause virtlockd killing libvirtd. Instead
of complicated code that would handle that, let's have a mutex
and keep the code simple.

The second interesting thing is keeping connection open between
lock and unlock API calls. This is achieved by duplicating client
FD and keeping it open until unlock is called. This trick is used
by regular disk content locking code when the FD is leaked to
qemu.

Finally, the third thing is polling implemented at client side.
Since virtlockd has only one thread that handles locking
requests, all it can do is either acquire lock or error out.
Therefore, the polling has to be implemented in client. The
polling is capped at 60 second timeout, which should be plenty
since the metadata lock is held only for a fraction of a second.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/security/security_manager.c | 135 ++++++++++++++++++++++++++++++++++++++++
 src/security/security_manager.h |   7 +++
 2 files changed, 142 insertions(+)

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 5c8370c159..dd5c3ac7e5 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -29,11 +29,15 @@
 #include "virobject.h"
 #include "virlog.h"
 #include "locking/lock_manager.h"
+#include "virfile.h"
+#include "virtime.h"
 
 #define VIR_FROM_THIS VIR_FROM_SECURITY
 
 VIR_LOG_INIT("security.security_manager");
 
+virMutex lockManagerMutex = VIR_MUTEX_INITIALIZER;
+
 struct _virSecurityManager {
     virObjectLockable parent;
 
@@ -43,6 +47,7 @@ struct _virSecurityManager {
     void *privateData;
 
     virLockManagerPluginPtr lockPlugin;
+    int fd;
 };
 
 static virClassPtr virSecurityManagerClass;
@@ -57,6 +62,7 @@ void virSecurityManagerDispose(void *obj)
         mgr->drv->close(mgr);
 
     virObjectUnref(mgr->lockPlugin);
+    VIR_FORCE_CLOSE(mgr->fd);
 
     VIR_FREE(mgr->privateData);
 }
@@ -109,6 +115,7 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv,
     mgr->flags = flags;
     mgr->virtDriver = virtDriver;
     VIR_STEAL_PTR(mgr->privateData, privateData);
+    mgr->fd = -1;
 
     if (drv->open(mgr) < 0)
         goto error;
@@ -1263,3 +1270,131 @@ virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
 
     return 0;
 }
+
+
+static virLockManagerPtr
+virSecurityManagerNewLockManager(virSecurityManagerPtr mgr,
+                                 const char * const *paths,
+                                 size_t npaths)
+{
+    virLockManagerPtr lock;
+    virLockManagerParam params[] = {
+        { .type = VIR_LOCK_MANAGER_PARAM_TYPE_UUID,
+            .key = "uuid",
+        },
+        { .type = VIR_LOCK_MANAGER_PARAM_TYPE_STRING,
+            .key = "name",
+            .value = { .cstr = "libvirtd-sec" },
+        },
+        { .type = VIR_LOCK_MANAGER_PARAM_TYPE_UINT,
+            .key = "pid",
+            .value = { .iv = getpid() },
+        },
+    };
+    const unsigned int flags = 0;
+    size_t i;
+
+    if (virGetHostUUID(params[0].value.uuid) < 0)
+        return NULL;
+
+    if (!(lock = virLockManagerNew(virLockManagerPluginGetDriver(mgr->lockPlugin),
+                                   VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON,
+                                   ARRAY_CARDINALITY(params),
+                                   params,
+                                   flags)))
+        return NULL;
+
+    for (i = 0; i < npaths; i++) {
+        if (virLockManagerAddResource(lock,
+                                      VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA,
+                                      paths[i], 0, NULL, 0) < 0)
+            goto error;
+    }
+
+    return lock;
+ error:
+    virLockManagerFree(lock);
+    return NULL;
+}
+
+
+/* How many seconds should we try to acquire the lock before
+ * giving up. */
+#define LOCK_ACQUIRE_TIMEOUT 60
+
+int
+virSecurityManagerMetadataLock(virSecurityManagerPtr mgr,
+                               const char * const *paths,
+                               size_t npaths)
+{
+    virLockManagerPtr lock;
+    virTimeBackOffVar timebackoff;
+    int fd = -1;
+    int rv;
+    int ret = -1;
+
+    virMutexLock(&lockManagerMutex);
+
+    if (!(lock = virSecurityManagerNewLockManager(mgr, paths, npaths)))
+        goto cleanup;
+
+    if (virTimeBackOffStart(&timebackoff, 1, LOCK_ACQUIRE_TIMEOUT * 1000) < 0)
+        goto cleanup;
+    while (virTimeBackOffWait(&timebackoff)) {
+        rv = virLockManagerAcquire(lock, NULL,
+                                   VIR_LOCK_MANAGER_ACQUIRE_ROLLBACK,
+                                   VIR_DOMAIN_LOCK_FAILURE_DEFAULT, &fd);
+
+        if (rv >= 0)
+            break;
+
+        if (virGetLastErrorCode() == VIR_ERR_RESOURCE_BUSY)
+            continue;
+
+        goto cleanup;
+    }
+
+    if (rv < 0)
+        goto cleanup;
+
+    mgr->fd = fd;
+    fd = -1;
+
+    ret = 0;
+ cleanup:
+    virLockManagerFree(lock);
+    VIR_FORCE_CLOSE(fd);
+    if (ret < 0)
+        virMutexUnlock(&lockManagerMutex);
+    return ret;
+}
+
+
+int
+virSecurityManagerMetadataUnlock(virSecurityManagerPtr mgr,
+                                 const char * const *paths,
+                                 size_t npaths)
+{
+    virLockManagerPtr lock;
+    int fd;
+    int ret = -1;
+
+    /* lockManagerMutex acquired from previous
+     * virSecurityManagerMetadataLock() call. */
+
+    fd = mgr->fd;
+    mgr->fd = -1;
+
+    if (!(lock = virSecurityManagerNewLockManager(mgr, paths, npaths)))
+        goto cleanup;
+
+    if (virLockManagerRelease(lock, NULL, 0) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    virLockManagerFree(lock);
+    VIR_FORCE_CLOSE(fd);
+    virMutexUnlock(&lockManagerMutex);
+    return ret;
+}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index c537e1c994..10ebe5cc29 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -199,4 +199,11 @@ int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr,
 int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
                                        virDomainDefPtr vm);
 
+int virSecurityManagerMetadataLock(virSecurityManagerPtr mgr,
+                                   const char * const *paths,
+                                   size_t npaths);
+int virSecurityManagerMetadataUnlock(virSecurityManagerPtr mgr,
+                                     const char * const *paths,
+                                     size_t npaths);
+
 #endif /* VIR_SECURITY_MANAGER_H__ */
-- 
2.16.4




More information about the libvir-list mailing list