[libvirt] [PATCH 1/2] 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(a)redhat.com>
---
src/security/security_dac.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index d457e6a..d7a2de4 100644
--- 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,32 @@ 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;
+ item = NULL;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(tmp);
+ VIR_FREE(item);
+ return ret;
}
static void
@@ -122,8 +132,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);
}
--
2.10.2