From 240ee7c1d8154004359a24397e29be6e03cb8e68 Mon Sep 17 00:00:00 2001 From: Andrea Bolognani Date: Fri, 16 Nov 2018 16:16:26 +0100 Subject: [PATCH] conf: Perform error checking in virDomainDeviceInfoFormat() virXMLFormatElement() might fail, but we were not checking its return value. Fixing this requires us to change virDomainDeviceInfoFormat() so that it can report an error back to the caller. Introduced-by: 0d6b87335c00451b0923ecc91d617f71e4135bf8 Spotted-by: Coverity Reported-by: John Ferlan Signed-off-by: Andrea Bolognani Reviewed-by: Pavel Hrdina --- src/conf/domain_conf.c | 97 ++++++++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1d04cac104..8cb9b2719c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6418,13 +6418,14 @@ virDomainVirtioOptionsFormat(virBufferPtr buf, } -static void ATTRIBUTE_NONNULL(2) +static int ATTRIBUTE_NONNULL(2) virDomainDeviceInfoFormat(virBufferPtr buf, virDomainDeviceInfoPtr info, unsigned int flags) { virBuffer attrBuf = VIR_BUFFER_INITIALIZER; virBuffer childBuf = VIR_BUFFER_INITIALIZER; + int ret = -1; if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) { virBufferAsprintf(buf, "bootIndex); @@ -6467,8 +6468,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf, } if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || - info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) - return; + info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { + /* We're done here */ + ret = 0; + goto cleanup; + } virBufferAsprintf(&attrBuf, " type='%s'", virDomainDeviceAddressTypeToString(info->type)); @@ -6562,10 +6566,16 @@ virDomainDeviceInfoFormat(virBufferPtr buf, break; } - virXMLFormatElement(buf, "address", &attrBuf, &childBuf); + if (virXMLFormatElement(buf, "address", &attrBuf, &childBuf) < 0) + goto cleanup; + ret = 0; + + cleanup: virBufferFreeAndReset(&attrBuf); virBufferFreeAndReset(&childBuf); + + return ret; } static int @@ -24299,8 +24309,10 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->src->encryption && !def->src->encryptionInherited && virStorageEncryptionFormat(buf, def->src->encryption) < 0) return -1; - virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); + if (virDomainDeviceInfoFormat(buf, &def->info, + flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) { + return -1; + } if (virDomainDiskDefFormatPrivateData(buf, def, flags, xmlopt) < 0) return -1; @@ -24475,7 +24487,8 @@ virDomainControllerDefFormat(virBufferPtr buf, virDomainControllerDriverFormat(&childBuf, def); - virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) + goto cleanup; if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && def->opts.pciopts.pcihole64) { @@ -24613,7 +24626,8 @@ virDomainFSDefFormat(virBufferPtr buf, if (def->readonly) virBufferAddLit(buf, "\n"); - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + goto cleanup; if (def->space_hard_limit) virBufferAsprintf(buf, "" @@ -25412,9 +25426,11 @@ virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefCoalesceFormatXML(buf, def->coalesce); - virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT - | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM); + if (virDomainDeviceInfoFormat(buf, &def->info, + flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT + | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) { + return -1; + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -25723,7 +25739,8 @@ virDomainChrDefFormat(virBufferPtr buf, if (virDomainChrTargetDefFormat(buf, def, flags) < 0) return -1; - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; virBufferAdjustIndent(buf, -2); virBufferAsprintf(buf, "\n", elementName); @@ -25772,7 +25789,8 @@ virDomainSmartcardDefFormat(virBufferPtr buf, _("unexpected smartcard type %d"), def->type); goto cleanup; } - virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) + goto cleanup; if (virBufferCheckError(&childBuf) < 0) goto cleanup; @@ -25843,7 +25861,8 @@ virDomainTPMDefFormat(virBufferPtr buf, break; } - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -25873,7 +25892,8 @@ virDomainSoundDefFormat(virBufferPtr buf, for (i = 0; i < def->ncodecs; i++) virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]); - virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) + goto cleanup; if (virBufferCheckError(&childBuf) < 0) goto cleanup; @@ -25922,7 +25942,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf, if (def->period) virBufferAsprintf(&childrenBuf, "\n", def->period); - virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) + goto cleanup; if (def->virtio) { virBuffer driverBuf = VIR_BUFFER_INITIALIZER; @@ -25965,7 +25986,8 @@ virDomainNVRAMDefFormat(virBufferPtr buf, { virBufferAddLit(buf, "\n"); virBufferAdjustIndent(buf, 2); - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -25998,7 +26020,8 @@ virDomainWatchdogDefFormat(virBufferPtr buf, goto cleanup; } - virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) + goto cleanup; if (virBufferCheckError(&childBuf) < 0) goto cleanup; @@ -26035,7 +26058,8 @@ static int virDomainPanicDefFormat(virBufferPtr buf, virDomainPanicModelTypeToString(def->model)); virBufferSetChildIndent(&childrenBuf, buf); - virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0); + if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0) + goto cleanup; if (virBufferCheckError(&childrenBuf) < 0) goto cleanup; @@ -26087,7 +26111,8 @@ virDomainShmemDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -26144,7 +26169,8 @@ virDomainRNGDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + goto cleanup; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -26271,7 +26297,8 @@ virDomainMemoryDefFormat(virBufferPtr buf, virDomainMemoryTargetDefFormat(buf, def); - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -26348,7 +26375,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - virDomainDeviceInfoFormat(buf, &def->info, flags); + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + goto cleanup; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -26404,7 +26432,8 @@ virDomainInputDefFormat(virBufferPtr buf, virBufferAddLit(&childbuf, "/>\n"); } virBufferEscapeString(&childbuf, "\n", def->source.evdev); - virDomainDeviceInfoFormat(&childbuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0) + goto cleanup; if (virBufferCheckError(&childbuf) < 0) goto cleanup; @@ -27004,9 +27033,11 @@ virDomainHostdevDefFormat(virBufferPtr buf, if (def->shareable) virBufferAddLit(buf, "\n"); - virDomainDeviceInfoFormat(buf, def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT - | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM); + if (virDomainDeviceInfoFormat(buf, def->info, + flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT + | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) { + return -1; + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); @@ -27032,8 +27063,10 @@ virDomainRedirdevDefFormat(virBufferPtr buf, if (virDomainChrSourceDefFormat(buf, def->source, flags) < 0) return -1; - virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); + if (virDomainDeviceInfoFormat(buf, &def->info, + flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) { + return -1; + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); return 0; @@ -27095,7 +27128,8 @@ virDomainHubDefFormat(virBufferPtr buf, goto cleanup; } - virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) + goto cleanup; if (virBufferCheckError(&childBuf) < 0) goto cleanup; @@ -27816,7 +27850,8 @@ virDomainVsockDefFormat(virBufferPtr buf, if (virXMLFormatElement(&childBuf, "cid", &cidAttrBuf, NULL) < 0) goto cleanup; - virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0); + if (virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0) < 0) + goto cleanup; if (virXMLFormatElement(buf, "vsock", &attrBuf, &childBuf) < 0) goto cleanup;