[libvirt] [PATCH v3 22/28] security_manager: Introduce metadata locking APIs
Michal Privoznik
mprivozn at redhat.com
Mon Aug 27 08:08:35 UTC 2018
Expose two APIs to lock and unlock metadata for given path. As
the comment from the header file says, this is somewhat
cumbersome, but it does not seem there is a better way.
The idea is that a security driver (like DAC or SELinux) will
call virSecurityManagerMetadataLock() just before they are about
to change the label followed by
virSecurityManagerMetadataUnlock() immediately after.
Now, because we can not make virlockd multithreaded (it uses
process associated POSIX locks where if one thread holds a lock
and another one open()+close() the same file it causes the lock
to be released), we can't have virtlockd to wait for the lock to
be set. There is just one thread so if that one waits for the
lock to be set there will not be another one coming to release
the lock. Therefore we have to implement 'try-set' at libvirtd
side. This is done by calling virLockManagerAcquire() in a loop
with possible usleep() until certain timeout is reached. Out of
thin air, the deadline was chosen to be 10 seconds with the
maximum sleeping time of 100 ms.
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
src/security/security_manager.c | 184 ++++++++++++++++++++++++++++++++++++++++
src/security/security_manager.h | 14 +++
2 files changed, 198 insertions(+)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 2238c75a5c..3ab06e0c4a 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -28,7 +28,10 @@
#include "viralloc.h"
#include "virobject.h"
#include "virlog.h"
+#include "virstring.h"
#include "locking/lock_manager.h"
+#include "virrandom.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -1389,3 +1392,184 @@ virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
return 0;
}
+
+
+static virLockManagerPtr
+virSecurityManagerNewLockManager(virSecurityManagerLockPtr mgrLock)
+{
+ 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;
+
+ if (virGetHostUUID(params[0].value.uuid) < 0)
+ return NULL;
+
+ if (!(lock = virLockManagerNew(virLockManagerPluginGetDriver(mgrLock->lockPlugin),
+ VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON,
+ ARRAY_CARDINALITY(params),
+ params,
+ flags)))
+ return NULL;
+
+ return lock;
+}
+
+
+/* How many miliseconds should we wait for the lock to be
+ * acquired before claiming error. */
+#define METADATA_LOCK_WAIT_MAX (10 * 1000)
+
+/* What is the maximum sleeping time (in miliseconds) between
+ * retries. */
+#define METADATA_LOCK_SLEEP_MAX (100)
+
+int
+virSecurityManagerMetadataLock(virSecurityManagerPtr mgr,
+ const char *path)
+{
+ virSecurityManagerLockPtr lock = mgr->lock;
+ unsigned long long now;
+ unsigned long long then;
+ int ret = -1;
+
+ VIR_DEBUG("mgr=%p path=%s lock=%p", mgr, path, lock);
+
+ if (!lock)
+ return 0;
+
+ virObjectLock(lock);
+
+ while (lock->pathLocked) {
+ if (virCondWait(&lock->cond, &lock->parent.lock) < 0) {
+ virReportSystemError(errno, "%s",
+ _("failed to wait on metadata condition"));
+ goto cleanup;
+ }
+ }
+
+ if (!lock->lock &&
+ !(lock->lock = virSecurityManagerNewLockManager(lock)))
+ goto cleanup;
+
+ if (virLockManagerAddResource(lock->lock,
+ VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA,
+ path, 0, NULL, 0) < 0)
+ goto cleanup;
+
+ if (virTimeMillisNowRaw(&now) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to get system time"));
+ goto cleanup;
+ }
+
+ then = now + METADATA_LOCK_WAIT_MAX;
+ while (1) {
+ uint32_t s;
+ int rc;
+
+ rc = virLockManagerAcquire(lock->lock, NULL,
+ VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN,
+ VIR_DOMAIN_LOCK_FAILURE_DEFAULT, NULL);
+
+ if (!rc)
+ break;
+
+ if (rc < 0) {
+ virErrorPtr err = virGetLastError();
+
+ if (err->code == VIR_ERR_SYSTEM_ERROR &&
+ err->int1 == EPIPE) {
+ /* Because we are sharing a connection, virtlockd
+ * might have been restarted and thus closed our
+ * connection. Retry. */
+ continue;
+ } else if (err->code != VIR_ERR_RESOURCE_BUSY) {
+ /* Some regular error. Exit now. */
+ goto cleanup;
+ }
+
+ /* Proceed to waiting & retry. */
+ }
+
+ if (now >= then)
+ goto cleanup;
+
+ s = virRandomInt(METADATA_LOCK_SLEEP_MAX) + 1;
+
+ if (now + s > then)
+ s = then - now;
+
+ usleep(1000 * s);
+
+ if (virTimeMillisNowRaw(&now) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to get system time"));
+ goto cleanup;
+ }
+ }
+
+ lock->pathLocked = true;
+ ret = 0;
+ cleanup:
+ if (lock->lock)
+ virLockManagerClearResources(lock->lock, 0);
+ if (ret < 0)
+ virSecurityManagerLockCloseConnLocked(lock, false);
+ virObjectUnlock(lock);
+ return ret;
+}
+
+
+int
+virSecurityManagerMetadataUnlock(virSecurityManagerPtr mgr,
+ const char *path)
+{
+ virSecurityManagerLockPtr lock = mgr->lock;
+ int ret = -1;
+
+ VIR_DEBUG("mgr=%p path=%s lock=%p", mgr, path, lock);
+
+ if (!lock)
+ return 0;
+
+ virObjectLock(lock);
+
+ /* Shouldn't happen, but doesn't hurt to check. */
+ if (!lock->lock) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unlock mismatch"));
+ goto cleanup;
+ }
+
+ if (virLockManagerAddResource(lock->lock,
+ VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA,
+ path, 0, NULL, 0) < 0)
+ goto cleanup;
+
+ if (virLockManagerRelease(lock->lock, NULL,
+ VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN) < 0)
+ goto cleanup;
+
+ lock->pathLocked = false;
+ virCondSignal(&lock->cond);
+ ret = 0;
+ cleanup:
+ if (lock->lock)
+ virLockManagerClearResources(lock->lock, 0);
+ if (ret < 0)
+ virSecurityManagerLockCloseConnLocked(lock, true);
+ virObjectUnlock(lock);
+ return ret;
+}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index c589b8808d..d6f36272eb 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -198,4 +198,18 @@ int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr,
int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
virDomainDefPtr vm);
+/* Ideally, these APIs wouldn't be here and the security manager
+ * would call lock and unlock from these APIs above just before
+ * calling corresponding callback from the driver. However, that
+ * means we would have to dig out paths from all the possible
+ * devices that APIs above handle which effectively means
+ * duplicating code from the driver (which has to do it already
+ * anyway).
+ * Therefore, have these APIs and let the driver call them when
+ * needed. */
+int virSecurityManagerMetadataLock(virSecurityManagerPtr mgr,
+ const char *path);
+int virSecurityManagerMetadataUnlock(virSecurityManagerPtr mgr,
+ const char *path);
+
#endif /* VIR_SECURITY_MANAGER_H__ */
--
2.16.4
More information about the libvir-list
mailing list