virSecurityManagerTransactionCommit: Do metadata locking iff enabled in config

When metadata locking is enabled that means the security commit
processing will be run in a fork similar to how namespaces use fork()'s
for processing. This is done to ensure libvirt can properly and
synchronously modify the metadata to store the original owner data.

Since fork()'s (e.g. virFork) have been seen as a performance bottleneck
being able to disable them allows the admin to choose whether the
performance 'hit' is worth the extra 'security' of being able to
remember the original owner of a lock.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

src/qemu/qemu_security.c

@@ -53,7 +53,8 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
                                       priv->chardevStdioLogd) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -86,7 +87,8 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
                                       priv->chardevStdioLogd);
 
     if (transactionStarted &&
-        virSecurityManagerTransactionCommit(driver->securityManager, -1) < 0)
+        virSecurityManagerTransactionCommit(driver->securityManager,
+                                            -1, priv->rememberOwner) < 0)
         VIR_WARN("Unable to run security manager transaction");
 
     virSecurityManagerTransactionAbort(driver->securityManager);
@@ -98,6 +100,7 @@ qemuSecuritySetDiskLabel(virQEMUDriverPtr driver,
                          virDomainObjPtr vm,
                          virDomainDiskDefPtr disk)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -112,7 +115,8 @@ qemuSecuritySetDiskLabel(virQEMUDriverPtr driver,
                                        disk) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -127,6 +131,7 @@ qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver,
                              virDomainObjPtr vm,
                              virDomainDiskDefPtr disk)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -141,7 +146,8 @@ qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver,
                                            disk) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -156,6 +162,7 @@ qemuSecuritySetImageLabel(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
                           virStorageSourcePtr src)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -170,7 +177,8 @@ qemuSecuritySetImageLabel(virQEMUDriverPtr driver,
                                         src) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -185,6 +193,7 @@ qemuSecurityRestoreImageLabel(virQEMUDriverPtr driver,
                               virDomainObjPtr vm,
                               virStorageSourcePtr src)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -199,7 +208,8 @@ qemuSecurityRestoreImageLabel(virQEMUDriverPtr driver,
                                             src) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -214,6 +224,7 @@ qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver,
                             virDomainObjPtr vm,
                             virDomainHostdevDefPtr hostdev)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -229,7 +240,8 @@ qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver,
                                           NULL) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -244,6 +256,7 @@ qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver,
                                 virDomainObjPtr vm,
                                 virDomainHostdevDefPtr hostdev)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -259,7 +272,8 @@ qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver,
                                               NULL) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -274,6 +288,7 @@ qemuSecuritySetMemoryLabel(virQEMUDriverPtr driver,
                            virDomainObjPtr vm,
                            virDomainMemoryDefPtr mem)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -288,7 +303,8 @@ qemuSecuritySetMemoryLabel(virQEMUDriverPtr driver,
                                          mem) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -303,6 +319,7 @@ qemuSecurityRestoreMemoryLabel(virQEMUDriverPtr driver,
                                virDomainObjPtr vm,
                                virDomainMemoryDefPtr mem)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -317,7 +334,8 @@ qemuSecurityRestoreMemoryLabel(virQEMUDriverPtr driver,
                                              mem) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -347,7 +365,8 @@ qemuSecuritySetInputLabel(virDomainObjPtr vm,
                                         input) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -377,7 +396,8 @@ qemuSecurityRestoreInputLabel(virDomainObjPtr vm,
                                             input) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -408,7 +428,8 @@ qemuSecuritySetChardevLabel(virQEMUDriverPtr driver,
                                           priv->chardevStdioLogd) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -439,7 +460,8 @@ qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
                                               priv->chardevStdioLogd) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -476,6 +498,7 @@ qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver,
                              int *exitstatus,
                              int *cmdret)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     int ret = -1;
     bool transactionStarted = false;
 
@@ -489,7 +512,8 @@ qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver,
         return -1;
     }
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, -1) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            -1, priv->rememberOwner) < 0)
         goto cleanup;
     transactionStarted = false;
 
@@ -522,7 +546,8 @@ qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver,
     virSecurityManagerRestoreTPMLabels(driver->securityManager, vm->def);
 
     if (transactionStarted &&
-        virSecurityManagerTransactionCommit(driver->securityManager, -1) < 0)
+        virSecurityManagerTransactionCommit(driver->securityManager,
+                                            -1, priv->rememberOwner) < 0)
         VIR_WARN("Unable to run security manager transaction");
 
     virSecurityManagerTransactionAbort(driver->securityManager);
@@ -534,6 +559,7 @@ void
 qemuSecurityCleanupTPMEmulator(virQEMUDriverPtr driver,
                                virDomainObjPtr vm)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     bool transactionStarted = false;
 
     if (virSecurityManagerTransactionStart(driver->securityManager) >= 0)
@@ -542,7 +568,8 @@ qemuSecurityCleanupTPMEmulator(virQEMUDriverPtr driver,
     virSecurityManagerRestoreTPMLabels(driver->securityManager, vm->def);
 
     if (transactionStarted &&
-        virSecurityManagerTransactionCommit(driver->securityManager, -1) < 0)
+        virSecurityManagerTransactionCommit(driver->securityManager,
+                                            -1, priv->rememberOwner) < 0)
         VIR_WARN("Unable to run security manager transaction");
 
     virSecurityManagerTransactionAbort(driver->securityManager);
@@ -555,6 +582,7 @@ qemuSecurityDomainSetPathLabel(virQEMUDriverPtr driver,
                                const char *path,
                                bool allowSubtree)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -570,7 +598,8 @@ qemuSecurityDomainSetPathLabel(virQEMUDriverPtr driver,
                                              allowSubtree) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -585,6 +614,7 @@ qemuSecuritySetSavedStateLabel(virQEMUDriverPtr driver,
                                    virDomainObjPtr vm,
                                    const char *savefile)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -599,7 +629,8 @@ qemuSecuritySetSavedStateLabel(virQEMUDriverPtr driver,
                                              savefile) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;
@@ -614,6 +645,7 @@ qemuSecurityRestoreSavedStateLabel(virQEMUDriverPtr driver,
                                        virDomainObjPtr vm,
                                        const char *savefile)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
     int ret = -1;
 
@@ -628,7 +660,8 @@ qemuSecurityRestoreSavedStateLabel(virQEMUDriverPtr driver,
                                                  savefile) < 0)
         goto cleanup;
 
-    if (virSecurityManagerTransactionCommit(driver->securityManager, pid) < 0)
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
         goto cleanup;
 
     ret = 0;

src/security/security_dac.c

@@ -86,6 +86,7 @@ struct _virSecurityDACChownList {
     virSecurityManagerPtr manager;
     virSecurityDACChownItemPtr *items;
     size_t nItems;
+    bool lock;
 };
 
 
@@ -210,22 +211,24 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
     int rv = 0;
     int ret = -1;
 
-    if (VIR_ALLOC_N(paths, list->nItems) < 0)
-        return -1;
+    if (list->lock) {
+        if (VIR_ALLOC_N(paths, list->nItems) < 0)
+            return -1;
 
-    for (i = 0; i < list->nItems; i++) {
-        const char *p = list->items[i]->path;
+        for (i = 0; i < list->nItems; i++) {
+            const char *p = list->items[i]->path;
 
-        if (!p ||
-            virFileIsDir(p))
-            continue;
+            if (!p ||
+                virFileIsDir(p))
+                continue;
 
-        VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p);
+            VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p);
+        }
+
+        if (virSecurityManagerMetadataLock(list->manager, paths, npaths) < 0)
+            goto cleanup;
     }
 
-    if (virSecurityManagerMetadataLock(list->manager, paths, npaths) < 0)
-        goto cleanup;
-
     for (i = 0; i < list->nItems; i++) {
         virSecurityDACChownItemPtr item = list->items[i];
 
@@ -246,7 +249,8 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
             break;
     }
 
-    if (virSecurityManagerMetadataUnlock(list->manager, paths, npaths) < 0)
+    if (list->lock &&
+        virSecurityManagerMetadataUnlock(list->manager, paths, npaths) < 0)
         goto cleanup;
 
     if (rv < 0)
@@ -529,11 +533,15 @@ virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
  * virSecurityDACTransactionCommit:
  * @mgr: security manager
  * @pid: domain's PID
+ * @lock: lock and unlock paths that are relabeled
  *
  * If @pid is not -1 then enter the @pid namespace (usually @pid refers
  * to a domain) and perform all the chown()-s on the list. If @pid is -1
  * then the transaction is performed in the namespace of the caller.
  *
+ * If @lock is true then all the paths that transaction would
+ * touch are locked before and unlocked after it is done so.
+ *
  * Note that the transaction is also freed, therefore new one has to be
  * started after successful return from this function. Also it is
  * considered as error if there's no transaction set and this function
@@ -544,9 +552,11 @@ virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
  */
 static int
 virSecurityDACTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
-                                pid_t pid)
+                                pid_t pid,
+                                bool lock)
 {
     virSecurityDACChownListPtr list;
+    int rc;
     int ret = -1;
 
     list = virThreadLocalGet(&chownList);
@@ -562,12 +572,20 @@ virSecurityDACTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    if ((pid == -1 &&
-         virSecurityDACTransactionRun(pid, list) < 0) ||
-        (pid != -1 &&
-         virProcessRunInMountNamespace(pid,
-                                       virSecurityDACTransactionRun,
-                                       list) < 0))
+    list->lock = lock;
+
+    if (pid == -1) {
+        if (lock)
+            rc = virProcessRunInFork(virSecurityDACTransactionRun, list);
+        else
+            rc = virSecurityDACTransactionRun(pid, list);
+    } else {
+        rc = virProcessRunInMountNamespace(pid,
+                                           virSecurityDACTransactionRun,
+                                           list);
+    }
+
+    if (rc < 0)
         goto cleanup;
 
     ret = 0;

src/security/security_driver.h

@@ -53,7 +53,8 @@ typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr);
 
 typedef int (*virSecurityDriverTransactionStart) (virSecurityManagerPtr mgr);
 typedef int (*virSecurityDriverTransactionCommit) (virSecurityManagerPtr mgr,
-                                                   pid_t pid);
+                                                   pid_t pid,
+                                                   bool lock);
 typedef void (*virSecurityDriverTransactionAbort) (virSecurityManagerPtr mgr);
 
 typedef int (*virSecurityDomainRestoreDiskLabel) (virSecurityManagerPtr mgr,

src/security/security_manager.c

@@ -299,12 +299,16 @@ virSecurityManagerTransactionStart(virSecurityManagerPtr mgr)
  * virSecurityManagerTransactionCommit:
  * @mgr: security manager
  * @pid: domain's PID
+ * @lock: lock and unlock paths that are relabeled
  *
  * If @pid is not -1 then enter the @pid namespace (usually @pid refers
  * to a domain) and perform all the operations on the transaction list.
  * If @pid is -1 then the transaction is performed in the namespace of
  * the caller.
  *
+ * If @lock is true then all the paths that transaction would
+ * touch are locked before and unlocked after it is done so.
+ *
  * Note that the transaction is also freed, therefore new one has to be
  * started after successful return from this function. Also it is
  * considered as error if there's no transaction set and this function
@@ -315,13 +319,14 @@ virSecurityManagerTransactionStart(virSecurityManagerPtr mgr)
  */
 int
 virSecurityManagerTransactionCommit(virSecurityManagerPtr mgr,
-                                    pid_t pid)
+                                    pid_t pid,
+                                    bool lock)
 {
     int ret = 0;
 
     virObjectLock(mgr);
     if (mgr->drv->transactionCommit)
-        ret = mgr->drv->transactionCommit(mgr, pid);
+        ret = mgr->drv->transactionCommit(mgr, pid, lock);
     virObjectUnlock(mgr);
     return ret;
 }

src/security/security_manager.h

@@ -79,7 +79,8 @@ void virSecurityManagerPostFork(virSecurityManagerPtr mgr);
 
 int virSecurityManagerTransactionStart(virSecurityManagerPtr mgr);
 int virSecurityManagerTransactionCommit(virSecurityManagerPtr mgr,
-                                        pid_t pid);
+                                        pid_t pid,
+                                        bool lock);
 void virSecurityManagerTransactionAbort(virSecurityManagerPtr mgr);
 
 void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr);

src/security/security_selinux.c

@@ -93,6 +93,7 @@ struct _virSecuritySELinuxContextList {
     virSecurityManagerPtr manager;
     virSecuritySELinuxContextItemPtr *items;
     size_t nItems;
+    bool lock;
 };
 
 #define SECURITY_SELINUX_VOID_DOI       "0"
@@ -221,21 +222,23 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
     int rv;
     int ret = -1;
 
-    if (VIR_ALLOC_N(paths, list->nItems) < 0)
-        return -1;
+    if (list->lock) {
+        if (VIR_ALLOC_N(paths, list->nItems) < 0)
+            return -1;
 
-    for (i = 0; i < list->nItems; i++) {
-        const char *p = list->items[i]->path;
+        for (i = 0; i < list->nItems; i++) {
+            const char *p = list->items[i]->path;
 
-        if (virFileIsDir(p))
-            continue;
+            if (virFileIsDir(p))
+                continue;
 
-        VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p);
+            VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p);
+        }
+
+        if (virSecurityManagerMetadataLock(list->manager, paths, npaths) < 0)
+            goto cleanup;
     }
 
-    if (virSecurityManagerMetadataLock(list->manager, paths, npaths) < 0)
-        goto cleanup;
-
     rv = 0;
     for (i = 0; i < list->nItems; i++) {
         virSecuritySELinuxContextItemPtr item = list->items[i];
@@ -250,7 +253,8 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
         }
     }
 
-    if (virSecurityManagerMetadataUnlock(list->manager, paths, npaths) < 0)
+    if (list->lock &&
+        virSecurityManagerMetadataUnlock(list->manager, paths, npaths) < 0)
         goto cleanup;
 
     if (rv < 0)
@@ -1072,12 +1076,16 @@ virSecuritySELinuxTransactionStart(virSecurityManagerPtr mgr)
  * virSecuritySELinuxTransactionCommit:
  * @mgr: security manager
  * @pid: domain's PID
+ * @lock: lock and unlock paths that are relabeled
  *
  * If @pis is not -1 then enter the @pid namespace (usually @pid refers
  * to a domain) and perform all the sefilecon()-s on the list. If @pid
  * is -1 then the transaction is performed in the namespace of the
  * caller.
  *
+ * If @lock is true then all the paths that transaction would
+ * touch are locked before and unlocked after it is done so.
+ *
  * Note that the transaction is also freed, therefore new one has to be
  * started after successful return from this function. Also it is
  * considered as error if there's no transaction set and this function
@@ -1088,9 +1096,11 @@ virSecuritySELinuxTransactionStart(virSecurityManagerPtr mgr)
  */
 static int
 virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
-                                    pid_t pid)
+                                    pid_t pid,
+                                    bool lock)
 {
     virSecuritySELinuxContextListPtr list;
+    int rc;
     int ret = -1;
 
     list = virThreadLocalGet(&contextList);
@@ -1106,12 +1116,20 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    if ((pid == -1 &&
-         virSecuritySELinuxTransactionRun(pid, list) < 0) ||
-        (pid != -1 &&
-         virProcessRunInMountNamespace(pid,
-                                       virSecuritySELinuxTransactionRun,
-                                       list) < 0))
+    list->lock = lock;
+
+    if (pid == -1) {
+        if (lock)
+            rc = virProcessRunInFork(virSecuritySELinuxTransactionRun, list);
+        else
+            rc = virSecuritySELinuxTransactionRun(pid, list);
+    } else {
+        rc = virProcessRunInMountNamespace(pid,
+                                           virSecuritySELinuxTransactionRun,
+                                           list);
+    }
+
+    if (rc < 0)
         goto cleanup;
 
     ret = 0;

src/security/security_stack.c

@@ -156,14 +156,15 @@ virSecurityStackTransactionStart(virSecurityManagerPtr mgr)
 
 static int
 virSecurityStackTransactionCommit(virSecurityManagerPtr mgr,
-                                  pid_t pid)
+                                  pid_t pid,
+                                  bool lock)
 {
     virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityStackItemPtr item = priv->itemsHead;
     int rc = 0;
 
     for (; item; item = item->next) {
-        if (virSecurityManagerTransactionCommit(item->securityManager, pid) < 0)
+        if (virSecurityManagerTransactionCommit(item->securityManager, pid, lock) < 0)
             rc = -1;
     }