[libvirt] [PATCH v4 17/23] security_dac: Move transaction handling up one level

[Michal Privoznik]

So far the whole transaction handling is done
virSecurityDACSetOwnershipInternal(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabelling it.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/security/security_dac.c | 65 ++++++++++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 21 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 926c9a33c1..52e28b5fda 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -77,12 +77,13 @@ struct _virSecurityDACChownItem {
     const virStorageSource *src;
     uid_t uid;
     gid_t gid;
+    bool restore;
 };
 
 typedef struct _virSecurityDACChownList virSecurityDACChownList;
 typedef virSecurityDACChownList *virSecurityDACChownListPtr;
 struct _virSecurityDACChownList {
-    virSecurityDACDataPtr priv;
+    virSecurityManagerPtr manager;
     virSecurityDACChownItemPtr *items;
     size_t nItems;
 };
@@ -95,7 +96,8 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
                               const char *path,
                               const virStorageSource *src,
                               uid_t uid,
-                              gid_t gid)
+                              gid_t gid,
+                              bool restore)
 {
     int ret = -1;
     char *tmp = NULL;
@@ -111,6 +113,7 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
     item->src = src;
     item->uid = uid;
     item->gid = gid;
+    item->restore = restore;
 
     if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0)
         goto cleanup;
@@ -159,25 +162,29 @@ static int
 virSecurityDACTransactionAppend(const char *path,
                                 const virStorageSource *src,
                                 uid_t uid,
-                                gid_t gid)
+                                gid_t gid,
+                                bool restore)
 {
     virSecurityDACChownListPtr list = virThreadLocalGet(&chownList);
     if (!list)
         return 0;
 
-    if (virSecurityDACChownListAppend(list, path, src, uid, gid) < 0)
+    if (virSecurityDACChownListAppend(list, path, src, uid, gid, restore) < 0)
         return -1;
 
     return 1;
 }
 
 
-static int virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
-                                              const virStorageSource *src,
-                                              const char *path,
-                                              uid_t uid,
-                                              gid_t gid);
+static int virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
+                                      const virStorageSource *src,
+                                      const char *path,
+                                      uid_t uid,
+                                      gid_t gid);
 
+static int virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
+                                                  const virStorageSource *src,
+                                                  const char *path);
 /**
  * virSecurityDACTransactionRun:
  * @pid: process pid
@@ -201,11 +208,16 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
         virSecurityDACChownItemPtr item = list->items[i];
 
         /* TODO Implement rollback */
-        if (virSecurityDACSetOwnershipInternal(list->priv,
-                                               item->src,
-                                               item->path,
-                                               item->uid,
-                                               item->gid) < 0)
+        if ((!item->restore &&
+             virSecurityDACSetOwnership(list->manager,
+                                        item->src,
+                                        item->path,
+                                        item->uid,
+                                        item->gid) < 0) ||
+            (item->restore &&
+             virSecurityDACRestoreFileLabelInternal(list->manager,
+                                                    item->src,
+                                                    item->path) < 0))
             return -1;
     }
 
@@ -455,7 +467,6 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
 static int
 virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
 {
-    virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     virSecurityDACChownListPtr list;
 
     list = virThreadLocalGet(&chownList);
@@ -468,7 +479,7 @@ virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
     if (VIR_ALLOC(list) < 0)
         return -1;
 
-    list->priv = priv;
+    list->manager = mgr;
 
     if (virThreadLocalSet(&chownList, list) < 0) {
         virReportSystemError(errno, "%s",
@@ -564,11 +575,6 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
     /* Be aware that this function might run in a separate process.
      * Therefore, any driver state changes would be thrown away. */
 
-    if ((rc = virSecurityDACTransactionAppend(path, src, uid, gid)) < 0)
-        return -1;
-    else if (rc > 0)
-        return 0;
-
     VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
              NULLSTR(src ? src->path : path), (long)uid, (long)gid);
 
@@ -640,11 +646,20 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
 {
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     struct stat sb;
+    int rc;
 
     if (!path && src && src->path &&
         virStorageSourceIsLocalStorage(src))
         path = src->path;
 
+    /* Be aware that this function might run in a separate process.
+     * Therefore, any driver state changes would be thrown away. */
+
+    if ((rc = virSecurityDACTransactionAppend(path, src, uid, gid, false)) < 0)
+        return -1;
+    else if (rc > 0)
+        return 0;
+
     if (path) {
         if (stat(path, &sb) < 0) {
             virReportSystemError(errno, _("unable to stat: %s"), path);
@@ -676,6 +691,14 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
         virStorageSourceIsLocalStorage(src))
         path = src->path;
 
+    /* Be aware that this function might run in a separate process.
+     * Therefore, any driver state changes would be thrown away. */
+
+    if ((rv = virSecurityDACTransactionAppend(path, src, uid, gid, true)) < 0)
+        return -1;
+    else if (rv > 0)
+        return 0;
+
     if (path) {
         rv = virSecurityDACRecallLabel(priv, path, &uid, &gid);
         if (rv < 0)
-- 
2.16.4