[libvirt] [PATCH v2 1/2] security_manager: Don't manipulate domain XML in virDomainDefGetSecurityLabelDef
Michal Privoznik
mprivozn at redhat.com
Thu Mar 21 15:35:10 UTC 2013
The virDomainDefGetSecurityLabelDef was modifying the domain XML.
It tried to find a seclabel corresponding to given sec driver. If the
label wasn't found, the function created one which is wrong. In fact
it's security manager which should modify this part of domain XML.
---
src/conf/domain_conf.c | 56 +++++++++++------------------------------
src/conf/domain_conf.h | 7 ++++--
src/libvirt_private.syms | 1 -
src/security/security_manager.c | 40 ++++++++++++++++++++---------
src/security/security_selinux.c | 8 ++++--
5 files changed, 53 insertions(+), 59 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b06cae5..8f5ae53 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1000,7 +1000,7 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def)
return;
}
-static void
+void
virSecurityLabelDefFree(virSecurityLabelDefPtr def)
{
if (!def)
@@ -1013,7 +1013,7 @@ virSecurityLabelDefFree(virSecurityLabelDefPtr def)
}
-static void
+void
virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
{
if (!def)
@@ -16616,10 +16616,6 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
return def->seclabels[i];
}
- seclabel = virDomainDefAddSecurityLabelDef(def, model);
- if (seclabel)
- seclabel->implicit = true;
-
return seclabel;
}
@@ -16654,55 +16650,31 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
}
virSecurityLabelDefPtr
-virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model)
+virDomainDefGenSecurityLabelDef(const char *model)
{
virSecurityLabelDefPtr seclabel = NULL;
- if (VIR_ALLOC(seclabel) < 0)
- goto no_memory;
-
- if (model) {
- seclabel->model = strdup(model);
- if (seclabel->model == NULL)
- goto no_memory;
+ if (VIR_ALLOC(seclabel) < 0 ||
+ (model && !(seclabel->model = strdup(model)))) {
+ virReportOOMError();
+ virSecurityLabelDefFree(seclabel);
+ seclabel = NULL;
}
- if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
- goto no_memory;
-
- def->seclabels[def->nseclabels - 1] = seclabel;
-
return seclabel;
-
-no_memory:
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
}
virSecurityDeviceLabelDefPtr
-virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
+virDomainDiskDefGenSecurityLabelDef(const char *model)
{
virSecurityDeviceLabelDefPtr seclabel = NULL;
- if (VIR_ALLOC(seclabel) < 0)
- goto no_memory;
-
- if (model) {
- seclabel->model = strdup(model);
- if (seclabel->model == NULL)
- goto no_memory;
+ if (VIR_ALLOC(seclabel) < 0 ||
+ (model && !(seclabel->model = strdup(model)))) {
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
+ seclabel = NULL;
}
- if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
- goto no_memory;
-
- def->seclabels[def->nseclabels - 1] = seclabel;
-
return seclabel;
-
-no_memory:
- virReportOOMError();
- virSecurityDeviceLabelDefFree(seclabel);
- return NULL;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a750a1f..2540bca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2297,10 +2297,13 @@ virSecurityDeviceLabelDefPtr
virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
virSecurityLabelDefPtr
-virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
+virDomainDefGenSecurityLabelDef(const char *model);
virSecurityDeviceLabelDefPtr
-virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+virDomainDiskDefGenSecurityLabelDef(const char *model);
+
+void virSecurityLabelDefFree(virSecurityLabelDefPtr def);
+void virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def);
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 21bc615..e8085a9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -108,7 +108,6 @@ virDomainControllerTypeToString;
virDomainCpuPlacementModeTypeFromString;
virDomainCpuPlacementModeTypeToString;
virDomainDefAddImplicitControllers;
-virDomainDefAddSecurityLabelDef;
virDomainDefCheckABIStability;
virDomainDefClearCCWAddresses;
virDomainDefClearDeviceAliases;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index c621366..5c2a95b 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -424,24 +424,26 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr,
int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm)
{
- int rc = 0;
+ int ret = -1;
size_t i;
virSecurityManagerPtr* sec_managers = NULL;
virSecurityLabelDefPtr seclabel;
+ bool generated = false;
if (mgr == NULL || mgr->drv == NULL)
- return -1;
+ return ret;
if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL)
- return -1;
+ return ret;
virObjectLock(mgr);
for (i = 0; sec_managers[i]; i++) {
- seclabel = virDomainDefGetSecurityLabelDef(vm,
- sec_managers[i]->drv->name);
- if (seclabel == NULL) {
- rc = -1;
- goto cleanup;
+ generated = false;
+ seclabel = virDomainDefGetSecurityLabelDef(vm, sec_managers[i]->drv->name);
+ if (!seclabel) {
+ if (!(seclabel = virDomainDefGenSecurityLabelDef(sec_managers[i]->drv->name)))
+ goto cleanup;
+ generated = seclabel->implicit = true;
}
if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) {
@@ -457,23 +459,37 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
sec_managers[i]->requireConfined) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Unconfined guests are not allowed on this host"));
- rc = -1;
goto cleanup;
}
if (!sec_managers[i]->drv->domainGenSecurityLabel) {
virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
} else {
- rc += sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm);
- if (rc)
+ /* The seclabel must be added to @vm prior calling domainGenSecurityLabel
+ * which may require seclabel to be presented already */
+
+ if (VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm) < 0) {
+ if (VIR_DELETE_ELEMENT(vm->seclabels,
+ vm->nseclabels -1, vm->nseclabels) < 0)
+ vm->nseclabels--;
goto cleanup;
+ }
}
}
+ ret = 0;
+
cleanup:
virObjectUnlock(mgr);
+ if (generated)
+ virSecurityLabelDefFree(seclabel);
VIR_FREE(sec_managers);
- return rc;
+ return ret;
}
int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 1e00637..60596ad 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1161,11 +1161,15 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
- disk_seclabel =
- virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
+ disk_seclabel = virDomainDiskDefGenSecurityLabelDef(SECURITY_SELINUX_NAME);
if (!disk_seclabel)
return -1;
disk_seclabel->norelabel = true;
+ if (VIR_APPEND_ELEMENT(disk->seclabels, disk->nseclabels, disk_seclabel) < 0) {
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(disk_seclabel);
+ return -1;
+ }
ret = 0;
}
return ret;
--
1.8.1.5
More information about the libvir-list
mailing list