From 48b49a631a0e6ab58376325c8301d5e52152a221 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 6 Feb 2013 12:40:41 +0000 Subject: [PATCH] Serialize execution of security manager APIs 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 --- 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 d4ec0f7842..f0a3da181c 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 a3f866972a..6f8ddbf74c 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; } -- GitLab