Re: [libvirt] [PATCH v3 1/5] Internal refactory of data structures

This patch updates the structures that store information about each domain and each hypervisor to support multiple security labels and drivers. It also updates all the remaining code to use the new fields. Signed-off-by: Marcelo Cerri <mhcerri(a)linux.vnet.ibm.com&gt; --- src/conf/capabilities.c | 17 ++++-- src/conf/capabilities.h | 6 ++- src/conf/domain_audit.c | 14 +++-- src/conf/domain_conf.c | 81 ++++++++++++++++------- src/conf/domain_conf.h | 9 ++- src/lxc/lxc_conf.c | 8 ++- src/lxc/lxc_controller.c | 8 +- src/lxc/lxc_driver.c | 11 ++-- src/lxc/lxc_process.c | 23 ++++--- src/qemu/qemu_driver.c | 15 +++-- src/qemu/qemu_process.c | 18 +++--- src/security/security_manager.c | 12 ++-- src/security/security_selinux.c | 133 ++++++++++++++++++++------------------- src/test/test_driver.c | 11 ++- 14 files changed, 216 insertions(+), 150 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 8b9b516..9a0bc3d 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -181,8 +181,13 @@ virCapabilitiesFree(virCapsPtr caps) { VIR_FREE(caps->host.migrateTrans); VIR_FREE(caps->host.arch); - VIR_FREE(caps->host.secModel.model); - VIR_FREE(caps->host.secModel.doi); + + for (i = 0; i < caps->host.nsecModels; i++) { + VIR_FREE(caps->host.secModels[i].model); + VIR_FREE(caps->host.secModels[i].doi); + } + VIR_FREE(caps->host.secModels); + virCPUDefFree(caps->host.cpu); VIR_FREE(caps); @@ -767,10 +772,12 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </topology>\n"); } - if (caps->host.secModel.model) { + if (caps->host.nsecModels) { virBufferAddLit(&xml, " <secmodel>\n"); - virBufferAsprintf(&xml, " <model>%s</model>\n", caps->host.secModel.model); - virBufferAsprintf(&xml, " <doi>%s</doi>\n", caps->host.secModel.doi); + virBufferAsprintf(&xml, " <model>%s</model>\n", + caps->host.secModels[0].model); + virBufferAsprintf(&xml, " <doi>%s</doi>\n", + caps->host.secModels[0].doi); virBufferAddLit(&xml, " </secmodel>\n"); } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 460b273..e43f404 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -93,6 +93,7 @@ struct _virCapsHostNUMACell { }; typedef struct _virCapsHostSecModel virCapsHostSecModel; +typedef virCapsHostSecModel *virCapsHostSecModelPtr; struct _virCapsHostSecModel { char *model; char *doi; @@ -116,7 +117,10 @@ struct _virCapsHost { size_t nnumaCell; size_t nnumaCell_max; virCapsHostNUMACellPtr *numaCell; - virCapsHostSecModel secModel; + + size_t nsecModels; + virCapsHostSecModelPtr secModels; + virCPUDefPtr cpu; unsigned char host_uuid[VIR_UUID_BUFLEN]; }; diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 5300371..59db649 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -618,6 +618,7 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) char uuidstr[VIR_UUID_STRING_BUFLEN]; char *vmname; const char *virt; + int i; virUUIDFormat(vm->def->uuid, uuidstr); if (!(vmname = virAuditEncode("vm", vm->def->name))) { @@ -630,11 +631,14 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) virt = "?"; } - VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, success, - "virt=%s %s uuid=%s vm-ctx=%s img-ctx=%s", - virt, vmname, uuidstr, - VIR_AUDIT_STR(vm->def->seclabel.label), - VIR_AUDIT_STR(vm->def->seclabel.imagelabel)); + for (i = 0; i < vm->def->nseclabels; i++) { + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, success, + "virt=%s %s uuid=%s vm-ctx=%s img-ctx=%s model=%s", + virt, vmname, uuidstr, + VIR_AUDIT_STR(vm->def->seclabels[i]->label), + VIR_AUDIT_STR(vm->def->seclabels[i]->imagelabel), + VIR_AUDIT_STR(vm->def->seclabels[i]->model)); + } VIR_FREE(vmname); } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58603a3..1c318a0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -855,12 +855,15 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def) } static void -virSecurityLabelDefClear(virSecurityLabelDefPtr def) +virSecurityLabelDefFree(virSecurityLabelDefPtr def) { + if (!def) + return; VIR_FREE(def->model); VIR_FREE(def->label); VIR_FREE(def->imagelabel); VIR_FREE(def->baselabel); + VIR_FREE(def); } @@ -869,6 +872,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) { if (!def) return; + VIR_FREE(def->model); VIR_FREE(def->label); VIR_FREE(def); } @@ -954,7 +958,11 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) virStorageEncryptionFree(def->encryption); virDomainDeviceInfoClear(&def->info); - virSecurityDeviceLabelDefFree(def->seclabel); + if (def->seclabels) { + for (i = 0; i < def->nseclabels; i++) + virSecurityDeviceLabelDefFree(def->seclabels[i]); + VIR_FREE(def->seclabels); + } for (i = 0 ; i < def->nhosts ; i++) virDomainDiskHostDefFree(&def->hosts[i]); @@ -1620,7 +1628,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainMemballoonDefFree(def->memballoon); - virSecurityLabelDefClear(&def->seclabel); + for (i = 0; i < def->nseclabels; i++) + virSecurityLabelDefFree(def->seclabels[i]); + VIR_FREE(def->seclabels); virCPUDefFree(def->cpu); @@ -3075,10 +3085,10 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, { char *p; - if (virXPathNode("./seclabel", ctxt) == NULL) + if (virXPathNode("./seclabel[1]", ctxt) == NULL) return 0; - p = virXPathStringLimit("string(./seclabel/@type)", + p = virXPathStringLimit("string(./seclabel[1]/@type)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; @@ -3092,7 +3102,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, } } - p = virXPathStringLimit("string(./seclabel/@relabel)", + p = virXPathStringLimit("string(./seclabel[1]/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3132,7 +3142,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (def->type == VIR_DOMAIN_SECLABEL_STATIC || (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel/label[1])", + p = virXPathStringLimit("string(./seclabel[1]/label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virReportError(VIR_ERR_XML_ERROR, @@ -3147,7 +3157,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (!def->norelabel && (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel/imagelabel[1])", + p = virXPathStringLimit("string(./seclabel[1]/imagelabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virReportError(VIR_ERR_XML_ERROR, @@ -3159,7 +3169,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, /* Only parse baselabel for dynamic label type */ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - p = virXPathStringLimit("string(./seclabel/baselabel[1])", + p = virXPathStringLimit("string(./seclabel[1]/baselabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); def->baselabel = p; } @@ -3171,7 +3181,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, def->baselabel || (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel/@model)", + p = virXPathStringLimit("string(./seclabel[1]/@model)", VIR_SECURITY_MODEL_BUFLEN-1, ctxt); if (p == NULL) { virReportError(VIR_ERR_XML_ERROR, @@ -3184,7 +3194,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, return 0; error: - virSecurityLabelDefClear(def); + virSecurityLabelDefFree(def); return -1; } @@ -3198,7 +3208,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, *def = NULL; - if (virXPathNode("./seclabel", ctxt) == NULL) + if (virXPathNode("./seclabel[1]", ctxt) == NULL) return 0; /* Can't use overrides if top-level doesn't allow relabeling. */ @@ -3214,7 +3224,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, return -1; } - p = virXPathStringLimit("string(./seclabel/@relabel)", + p = virXPathStringLimit("string(./seclabel[1]/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3233,7 +3243,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, (*def)->norelabel = false; } - p = virXPathStringLimit("string(./seclabel/label[1])", + p = virXPathStringLimit("string(./seclabel[1]/label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); (*def)->label = p; @@ -3667,10 +3677,16 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if (virSecurityDeviceLabelDefParseXML(&def->seclabel, + if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0]) < 0)) { + virReportOOMError(); + goto error; + } + if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0], vmSeclabel, ctxt) < 0) goto error; + def->nseclabels = 1; ctxt->node = saved_node; } @@ -7120,10 +7136,17 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } + if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0])) < 0 ) { + virReportOOMError(); + goto error; + } +

if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, &def->seclabel, flags))) + NULL, def->seclabels[0], + flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -8061,7 +8084,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ - if (virSecurityLabelDefParseXML(&def->seclabel, ctxt, flags) == -1) + if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0]) < 0)) { + virReportOOMError(); + goto error; + } + def->nseclabels = 1; + if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1) goto error; /* Extract domain memory */ @@ -8661,7 +8690,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, - &def->seclabel, + def->seclabels[0], flags); if (!disk) goto error; @@ -11181,10 +11210,10 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->seclabel) { + if (def->seclabels && def->seclabels[0]) {

virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabel); + virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -11194,10 +11223,10 @@ virDomainDiskDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", def->src); - if (def->seclabel) { + if (def->seclabels && def->seclabels[0]) {

virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabel); + virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else {