[libvirt] [PATCH v3 22/28] security_manager: Introduce metadata locking APIs

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(a)redhat.com&gt; --- 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