On Tue, Jan 04, 2022 at 09:14:29 +0100, Michal Privoznik wrote:
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 53 ++++++++------------ tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 1 - 2 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 716c6d2240..b8fef8586c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25481,63 +25481,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) backendBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
Cannonically this would be 'backendChildBuf'. [...] Above code: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml index 59dd68311f..79acde218b 100644 --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -30,7 +30,6 @@ <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> - > <active_pcr_banks> <sha256/> <sha512/> -- 2.34.1