[libvirt] [PATCH 2/4] Serialize execution of security manager APIs

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Add locking to virSecurityManagerXXX APIs, so that use of the security drivers is internally serialized. This avoids the need to rely on the global driver locks to achieve serialization Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/qemu/qemu_conf.h | 2 +- src/security/security_manager.c | 200 +++++++++++++++++++++++++++++++--------- 2 files changed, 157 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d4ec0f7..f0a3da1 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -194,7 +194,7 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virDomainEventStatePtr domainEventState; - /* Immutable pointer. Unsafe APIs. XXX */ + /* Immutable pointer. self-locking APIs */ virSecurityManagerPtr securityManager; /* Immutable pointers. Requires locks to be held before diff --git a/src/security/security_manager.c b/src/security/security_manager.c index a3f8669..6f8ddbf 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -216,8 +216,13 @@ virSecurityManagerGetDriver(virSecurityManagerPtr mgr) const char * virSecurityManagerGetDOI(virSecurityManagerPtr mgr) { - if (mgr->drv->getDOI) - return mgr->drv->getDOI(mgr); + if (mgr->drv->getDOI) { + const char *ret; + virObjectLock(mgr); + ret = mgr->drv->getDOI(mgr); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; @@ -226,8 +231,13 @@ virSecurityManagerGetDOI(virSecurityManagerPtr mgr) const char * virSecurityManagerGetModel(virSecurityManagerPtr mgr) { - if (mgr->drv->getModel) - return mgr->drv->getModel(mgr); + if (mgr->drv->getModel) { + const char *ret; + virObjectLock(mgr); + ret = mgr->drv->getModel(mgr); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; @@ -252,8 +262,13 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virDomainDiskDefPtr disk) { - if (mgr->drv->domainRestoreSecurityImageLabel) - return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk); + if (mgr->drv->domainRestoreSecurityImageLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -262,8 +277,13 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (mgr->drv->domainSetSecurityDaemonSocketLabel) - return mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm); + if (mgr->drv->domainSetSecurityDaemonSocketLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -272,8 +292,13 @@ int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (mgr->drv->domainSetSecuritySocketLabel) - return mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + if (mgr->drv->domainSetSecuritySocketLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -282,8 +307,13 @@ int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (mgr->drv->domainClearSecuritySocketLabel) - return mgr->drv->domainClearSecuritySocketLabel(mgr, vm); + if (mgr->drv->domainClearSecuritySocketLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainClearSecuritySocketLabel(mgr, vm); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -293,8 +323,13 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virDomainDiskDefPtr disk) { - if (mgr->drv->domainSetSecurityImageLabel) - return mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk); + if (mgr->drv->domainSetSecurityImageLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -305,8 +340,13 @@ int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev, const char *vroot) { - if (mgr->drv->domainRestoreSecurityHostdevLabel) - return mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot); + if (mgr->drv->domainRestoreSecurityHostdevLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -317,8 +357,13 @@ int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev, const char *vroot) { - if (mgr->drv->domainSetSecurityHostdevLabel) - return mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot); + if (mgr->drv->domainSetSecurityHostdevLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -328,8 +373,13 @@ int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *savefile) { - if (mgr->drv->domainSetSavedStateLabel) - return mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile); + if (mgr->drv->domainSetSavedStateLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -339,8 +389,13 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *savefile) { - if (mgr->drv->domainRestoreSavedStateLabel) - return mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile); + if (mgr->drv->domainRestoreSavedStateLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -360,6 +415,7 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL) return -1; + virObjectLock(mgr); for (i = 0; sec_managers[i]; i++) { seclabel = virDomainDefGetSecurityLabelDef(vm, sec_managers[i]->drv->name); @@ -395,6 +451,7 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, } cleanup: + virObjectUnlock(mgr); VIR_FREE(sec_managers); return rc; } @@ -403,8 +460,13 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, pid_t pid) { - if (mgr->drv->domainReserveSecurityLabel) - return mgr->drv->domainReserveSecurityLabel(mgr, vm, pid); + if (mgr->drv->domainReserveSecurityLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainReserveSecurityLabel(mgr, vm, pid); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -413,8 +475,13 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (mgr->drv->domainReleaseSecurityLabel) - return mgr->drv->domainReleaseSecurityLabel(mgr, vm); + if (mgr->drv->domainReleaseSecurityLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainReleaseSecurityLabel(mgr, vm); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -424,8 +491,13 @@ int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *stdin_path) { - if (mgr->drv->domainSetSecurityAllLabel) - return mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path); + if (mgr->drv->domainSetSecurityAllLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -435,8 +507,13 @@ int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, int migrated) { - if (mgr->drv->domainRestoreSecurityAllLabel) - return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated); + if (mgr->drv->domainRestoreSecurityAllLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -447,8 +524,13 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, pid_t pid, virSecurityLabelPtr sec) { - if (mgr->drv->domainGetSecurityProcessLabel) - return mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec); + if (mgr->drv->domainGetSecurityProcessLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -457,8 +539,13 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (mgr->drv->domainSetSecurityProcessLabel) - return mgr->drv->domainSetSecurityProcessLabel(mgr, vm); + if (mgr->drv->domainSetSecurityProcessLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityProcessLabel(mgr, vm); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -480,8 +567,13 @@ int virSecurityManagerVerify(virSecurityManagerPtr mgr, if (secdef == NULL || secdef->model == NULL) return 0; - if (mgr->drv->domainSecurityVerify) - return mgr->drv->domainSecurityVerify(mgr, def); + if (mgr->drv->domainSecurityVerify) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSecurityVerify(mgr, def); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -491,8 +583,13 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, int fd) { - if (mgr->drv->domainSetSecurityImageFDLabel) - return mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd); + if (mgr->drv->domainSetSecurityImageFDLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -502,8 +599,13 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, int fd) { - if (mgr->drv->domainSetSecurityTapFDLabel) - return mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd); + if (mgr->drv->domainSetSecurityTapFDLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; @@ -512,8 +614,13 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (mgr->drv->domainGetSecurityMountOptions) - return mgr->drv->domainGetSecurityMountOptions(mgr, vm); + if (mgr->drv->domainGetSecurityMountOptions) { + char *ret; + virObjectLock(mgr); + ret = mgr->drv->domainGetSecurityMountOptions(mgr, vm); + virObjectUnlock(mgr); + return ret; + } virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; @@ -542,8 +649,13 @@ int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *path) { - if (mgr->drv->domainSetSecurityHugepages) - return mgr->drv->domainSetSecurityHugepages(mgr, vm, path); + if (mgr->drv->domainSetSecurityHugepages) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityHugepages(mgr, vm, path); + virObjectUnlock(mgr); + return ret; + } return 0; } -- 1.8.1