security: DAC: fix the transaction model's list append

The problem is in the way how the list item is created prior to
appending it to the transaction list - the @path attribute is just a
shallow copy instead of deep copy of the hostdev device's path.
Unfortunately, the hostdev devices from which the @path is extracted, in
order to add them into the transaction list, are only temporary and
freed before the buildup of the qemu namespace, thus making the @path
attribute in the transaction list NULL, causing 'permission denied' or
'double free' or 'unknown cause' errors.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1413773

Signed-off-by: Erik Skultety <eskultet@redhat.com>

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index d457e6aa2a061900009e13634c06f381db36c754..67219170c0a73f3eca417d73236cfb0ec0f46c8f 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -71,7 +71,7 @@ struct _virSecurityDACCallbackData {
 typedef struct _virSecurityDACChownItem virSecurityDACChownItem;
 typedef virSecurityDACChownItem *virSecurityDACChownItemPtr;
 struct _virSecurityDACChownItem {
-    const char *path;
+    char *path;
     const virStorageSource *src;
     uid_t uid;
     gid_t gid;
@@ -95,22 +95,31 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
                               uid_t uid,
                               gid_t gid)
 {
-    virSecurityDACChownItemPtr item;
+    int ret = -1;
+    char *tmp = NULL;
+    virSecurityDACChownItemPtr item = NULL;
 
     if (VIR_ALLOC(item) < 0)
         return -1;
 
-    item->path = path;
+    if (VIR_STRDUP(tmp, path) < 0)
+        goto cleanup;
+
+    item->path = tmp;
     item->src = src;
     item->uid = uid;
     item->gid = gid;
 
-    if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0) {
-        VIR_FREE(item);
-        return -1;
-    }
+    if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0)
+        goto cleanup;
 
-    return 0;
+    tmp = NULL;
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(tmp);
+    VIR_FREE(item);
+    return ret;
 }
 
 static void
@@ -122,8 +131,10 @@ virSecurityDACChownListFree(void *opaque)
     if (!list)
         return;
 
-    for (i = 0; i < list->nItems; i++)
+    for (i = 0; i < list->nItems; i++) {
+        VIR_FREE(list->items[i]->path);
         VIR_FREE(list->items[i]);
+    }
     VIR_FREE(list);
 }