[libvirt] [PATCH v3 20/28] security_manager: Load lock plugin on init

Michal Privoznik mprivozn at redhat.com
Mon Aug 27 08:08:33 UTC 2018


When creating the security managers stack load the lock plugin
too. This is done by creating a single object that all secdrivers
take a reference to. We have to have one shared object so that
the connection to virlockd can be shared between individual
secdrivers. It is important that the connection is shared because
if the connection is closed from one driver while other has a
file locked, then virtlockd does its job and kills libvirtd.

The cfg.mk change is needed in order to allow syntax-check
to include lock_manager.h. This is generally safe thing to do as
this APIs defined there will always exist. However, instead of
allowing the include for all other drivers (like cpu, network,
and so on) allow it only for security driver. This will still
trigger the error if including from other drivers.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 cfg.mk                          |  4 +-
 src/qemu/qemu_driver.c          | 12 ++++--
 src/security/security_manager.c | 81 ++++++++++++++++++++++++++++++++++++++++-
 src/security/security_manager.h |  3 +-
 tests/testutilsqemu.c           |  2 +-
 5 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 609ae869c2..e0a7b5105a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -787,8 +787,10 @@ sc_prohibit_cross_inclusion:
 	  case $$dir in \
 	    util/) safe="util";; \
 	    access/ | conf/) safe="($$dir|conf|util)";; \
-	    cpu/| network/| node_device/| rpc/| security/| storage/) \
+	    cpu/| network/| node_device/| rpc/| storage/) \
 	      safe="($$dir|util|conf|storage)";; \
+	    security/) \
+	      safe="($$dir|util|conf|storage|locking)";; \
 	    xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
 	    *) safe="($$dir|$(mid_dirs)|util)";; \
 	  esac; \
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index da8c4e8991..e06dee8dfb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -358,7 +358,9 @@ qemuSecurityInit(virQEMUDriverPtr driver)
                                         flags)))
                 goto error;
             if (!stack) {
-                if (!(stack = qemuSecurityNewStack(mgr)))
+                if (!(stack = qemuSecurityNewStack(mgr,
+                                                   cfg->metadataLockManagerName ?
+                                                   cfg->metadataLockManagerName : "nop")))
                     goto error;
             } else {
                 if (qemuSecurityStackAddNested(stack, mgr) < 0)
@@ -372,7 +374,9 @@ qemuSecurityInit(virQEMUDriverPtr driver)
                                     QEMU_DRIVER_NAME,
                                     flags)))
             goto error;
-        if (!(stack = qemuSecurityNewStack(mgr)))
+        if (!(stack = qemuSecurityNewStack(mgr,
+                                           cfg->metadataLockManagerName ?
+                                           cfg->metadataLockManagerName : "nop")))
             goto error;
         mgr = NULL;
     }
@@ -389,7 +393,9 @@ qemuSecurityInit(virQEMUDriverPtr driver)
                                        qemuSecurityChownCallback)))
             goto error;
         if (!stack) {
-            if (!(stack = qemuSecurityNewStack(mgr)))
+            if (!(stack = qemuSecurityNewStack(mgr,
+                                               cfg->metadataLockManagerName ?
+                                               cfg->metadataLockManagerName : "nop")))
                 goto error;
         } else {
             if (qemuSecurityStackAddNested(stack, mgr) < 0)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 21eb6f7452..caaff1f703 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -28,21 +28,39 @@
 #include "viralloc.h"
 #include "virobject.h"
 #include "virlog.h"
+#include "locking/lock_manager.h"
 
 #define VIR_FROM_THIS VIR_FROM_SECURITY
 
 VIR_LOG_INIT("security.security_manager");
 
+typedef struct _virSecurityManagerLock virSecurityManagerLock;
+typedef virSecurityManagerLock *virSecurityManagerLockPtr;
+struct _virSecurityManagerLock {
+    virObjectLockable parent;
+
+    virCond cond;
+
+    virLockManagerPluginPtr lockPlugin;
+    virLockManagerPtr lock;
+
+    bool pathLocked;
+};
+
 struct _virSecurityManager {
     virObjectLockable parent;
 
     virSecurityDriverPtr drv;
     unsigned int flags;
     const char *virtDriver;
+
+    virSecurityManagerLockPtr lock;
+
     void *privateData;
 };
 
 static virClassPtr virSecurityManagerClass;
+static virClassPtr virSecurityManagerLockClass;
 
 
 static
@@ -52,16 +70,36 @@ void virSecurityManagerDispose(void *obj)
 
     if (mgr->drv->close)
         mgr->drv->close(mgr);
+
+    virObjectUnref(mgr->lock);
+
     VIR_FREE(mgr->privateData);
 }
 
 
+static void
+virSecurityManagerLockDispose(void *obj)
+{
+    virSecurityManagerLockPtr lock = obj;
+
+    virCondDestroy(&lock->cond);
+
+    if (lock->lock)
+        virLockManagerCloseConn(lock->lock, 0);
+    virLockManagerFree(lock->lock);
+    virLockManagerPluginUnref(lock->lockPlugin);
+}
+
+
 static int
 virSecurityManagerOnceInit(void)
 {
     if (!VIR_CLASS_NEW(virSecurityManager, virClassForObjectLockable()))
         return -1;
 
+    if (!VIR_CLASS_NEW(virSecurityManagerLock, virClassForObjectLockable()))
+        return -1;
+
     return 0;
 }
 
@@ -106,8 +144,32 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv,
 }
 
 
+static virSecurityManagerLockPtr
+virSecurityManagerLockNew(const char *lockManagerName)
+{
+    virSecurityManagerLockPtr ret;
+
+    if (!(ret = virObjectLockableNew(virSecurityManagerLockClass)))
+        return NULL;
+
+    if (virCondInit(&ret->cond) < 0)
+        goto error;
+
+    if (!(ret->lockPlugin = virLockManagerPluginNew(lockManagerName,
+                                                    NULL, NULL, 0))) {
+        goto error;
+    }
+
+    return ret;
+ error:
+    virObjectUnref(ret);
+    return NULL;
+}
+
+
 virSecurityManagerPtr
-virSecurityManagerNewStack(virSecurityManagerPtr primary)
+virSecurityManagerNewStack(virSecurityManagerPtr primary,
+                           const char *lockManagerName)
 {
     virSecurityManagerPtr mgr =
         virSecurityManagerNewDriver(&virSecurityDriverStack,
@@ -117,9 +179,16 @@ virSecurityManagerNewStack(virSecurityManagerPtr primary)
     if (!mgr)
         return NULL;
 
+    if (!(mgr->lock = virSecurityManagerLockNew(lockManagerName)))
+        goto error;
+
     if (virSecurityStackAddNested(mgr, primary) < 0)
         goto error;
 
+    /* Propagate lock manager */
+    if (!primary->lock)
+        primary->lock = virObjectRef(mgr->lock);
+
     return mgr;
  error:
     virObjectUnref(mgr);
@@ -133,7 +202,15 @@ virSecurityManagerStackAddNested(virSecurityManagerPtr stack,
 {
     if (STRNEQ("stack", stack->drv->name))
         return -1;
-    return virSecurityStackAddNested(stack, nested);
+
+    if (virSecurityStackAddNested(stack, nested) < 0)
+        return -1;
+
+    /* Propagate lock manager */
+    if (!nested->lock)
+        nested->lock = virObjectRef(stack->lock);
+
+    return 0;
 }
 
 
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 1ead369e82..c589b8808d 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -47,7 +47,8 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name,
                                             const char *virtDriver,
                                             unsigned int flags);
 
-virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary);
+virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary,
+                                                 const char *lockManagerName);
 int virSecurityManagerStackAddNested(virSecurityManagerPtr stack,
                                      virSecurityManagerPtr nested);
 
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 8438613f28..2a2a88361b 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -721,7 +721,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
     if (!(mgr = virSecurityManagerNew("none", "qemu",
                                       VIR_SECURITY_MANAGER_PRIVILEGED)))
         goto error;
-    if (!(driver->securityManager = virSecurityManagerNewStack(mgr)))
+    if (!(driver->securityManager = virSecurityManagerNewStack(mgr, "nop")))
         goto error;
 
     return 0;
-- 
2.16.4




More information about the libvir-list mailing list