The <tpm/> element formatting is handled in virDomainTPMDefFormat() which uses the "old style" - appending strings directly into the output buffer. With this, it's easy to get conditions that tell when an element has ended wrong. In this particular case, if both <encryption/> and <active_pcr_banks/> are to be formatted the current code puts a stray '>' into the output buffer, resulting in invalid XML. Rewrite the function to use virXMLFormatElement() which is more clever. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15 Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- src/conf/domain_conf.c | 53 ++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bba662bf4c..9e854d031e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25495,63 +25495,54 @@ virDomainTPMDefFormat(virBuffer *buf, virDomainTPMDef *def, unsigned int flags) { - virBufferAsprintf(buf, "<tpm model='%s'>\n", + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) backendAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) backendChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); + + virBufferAsprintf(&attrBuf, " model='%s'", virDomainTPMModelTypeToString(def->model)); - virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<backend type='%s'", + + virBufferAsprintf(&backendAttrBuf, " type='%s'", virDomainTPMBackendTypeToString(def->type)); switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<device path='%s'/>\n", + virBufferEscapeString(&backendChildBuf, "<device path='%s'/>\n", def->data.passthrough.source->data.file.path); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</backend>\n"); break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: - virBufferAsprintf(buf, " version='%s'", + virBufferAsprintf(&backendAttrBuf, " version='%s'", virDomainTPMVersionTypeToString(def->version)); if (def->data.emulator.persistent_state) - virBufferAddLit(buf, " persistent_state='yes'"); + virBufferAddLit(&backendAttrBuf, " persistent_state='yes'"); if (def->data.emulator.hassecretuuid) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<encryption secret='%s'/>\n", - virUUIDFormat(def->data.emulator.secretuuid, uuidstr)); - virBufferAdjustIndent(buf, -2); + + virBufferAsprintf(&backendChildBuf, "<encryption secret='%s'/>\n", + virUUIDFormat(def->data.emulator.secretuuid, uuidstr)); } if (def->data.emulator.activePcrBanks) { + g_auto(virBuffer) activePcrBanksBuf = VIR_BUFFER_INIT_CHILD(&backendChildBuf); size_t i; - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferAddLit(buf, "<active_pcr_banks>\n"); - virBufferAdjustIndent(buf, 2); + for (i = VIR_DOMAIN_TPM_PCR_BANK_SHA1; i < VIR_DOMAIN_TPM_PCR_BANK_LAST; i++) { if ((def->data.emulator.activePcrBanks & (1 << i))) - virBufferAsprintf(buf, "<%s/>\n", + virBufferAsprintf(&activePcrBanksBuf, "<%s/>\n", virDomainTPMPcrBankTypeToString(i)); } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</active_pcr_banks>\n"); - virBufferAdjustIndent(buf, -2); + + virXMLFormatElement(&backendChildBuf, "active_pcr_banks", NULL, &activePcrBanksBuf); } - if (def->data.emulator.hassecretuuid || - def->data.emulator.activePcrBanks) - virBufferAddLit(buf, "</backend>\n"); - else - virBufferAddLit(buf, "/>\n"); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } - virDomainDeviceInfoFormat(buf, &def->info, flags); + virXMLFormatElement(&childBuf, "backend", &backendAttrBuf, &backendChildBuf); + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</tpm>\n"); + virXMLFormatElement(buf, "tpm", &attrBuf, &childBuf); return 0; } -- 2.34.1