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 <berrange@redhat.com>

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d4ec0f7842befe4bfd1299a5d5d63b8613c0d150..f0a3da181c58fd2ea3788fae8107b7415d37a68f 100644 (file)
--- 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 a3f866972a2334171a07bbd3a5fbbdc503933cf6..6f8ddbf74c0be24b62bb7c161af0fdadcfc8b3fa 100644 (file)
--- 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;
 }