From 7e8b2da74f1322050a993ca988bfbea997a84355 Mon Sep 17 00:00:00 2001
From: Erik Skultety <eskultet@redhat.com>
Date: Tue, 17 Jan 2017 12:22:14 +0100
Subject: [PATCH] security: SELinux: 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 argument 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>
---
src/security/security_selinux.c | 38 +++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index f229b5139a..e22de0653f 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -81,8 +81,8 @@ struct _virSecuritySELinuxCallbackData {
typedef struct _virSecuritySELinuxContextItem virSecuritySELinuxContextItem;
typedef virSecuritySELinuxContextItem *virSecuritySELinuxContextItemPtr;
struct _virSecuritySELinuxContextItem {
- const char *path;
- const char *tcon;
+ char *path;
+ char *tcon;
bool optional;
};
@@ -105,27 +105,42 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr,
virThreadLocal contextList;
+
+static void
+virSecuritySELinuxContextItemFree(virSecuritySELinuxContextItemPtr item)
+{
+ if (!item)
+ return;
+
+ VIR_FREE(item->path);
+ VIR_FREE(item->tcon);
+ VIR_FREE(item);
+}
+
static int
virSecuritySELinuxContextListAppend(virSecuritySELinuxContextListPtr list,
const char *path,
const char *tcon,
bool optional)
{
- virSecuritySELinuxContextItemPtr item;
+ int ret = -1;
+ virSecuritySELinuxContextItemPtr item = NULL;
if (VIR_ALLOC(item) < 0)
return -1;
- item->path = path;
- item->tcon = tcon;
+ if (VIR_STRDUP(item->path, path) < 0 || VIR_STRDUP(item->tcon, tcon) < 0)
+ goto cleanup;
+
item->optional = optional;
- 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;
+ ret = 0;
+ cleanup:
+ virSecuritySELinuxContextItemFree(item);
+ return ret;
}
static void
@@ -138,7 +153,8 @@ virSecuritySELinuxContextListFree(void *opaque)
return;
for (i = 0; i < list->nItems; i++)
- VIR_FREE(list->items[i]);
+ virSecuritySELinuxContextItemFree(list->items[i]);
+
VIR_FREE(list);
}