[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v3 27/28] security_dac: Move transaction handling up one level



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 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 7528d8ba7d..2115e00e07 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",
@@ -558,11 +569,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;
-
     if (priv && src && priv->chownCallback) {
         rc = priv->chownCallback(src, uid, gid);
         /* here path is used only for error messages */
@@ -631,11 +637,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);
@@ -667,6 +682,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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]