[libvirt] [PATCH v2 0/4] Misc fixes to network port code

Fixes for bugs spotted by coverity Daniel P. Berrangé (4): conf: fix leak when parsing network port XML conf: fix leak of directory handle when loading network ports conf: fix NULL deref when exporting ports conf: add error checking of UUID generation src/conf/domain_conf.c | 12 ++++++++++-- src/conf/virnetworkobj.c | 9 ++++++--- src/conf/virnetworkportdef.c | 26 +++++++++----------------- 3 files changed, 25 insertions(+), 22 deletions(-) -- 2.21.0

Use auto free to avoid leaking the "trustGuestRxFilters" strings Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/virnetworkportdef.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 379411e088..5dfa3a3583 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -76,19 +76,19 @@ static virNetworkPortDefPtr virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) { virNetworkPortDefPtr def; - char *uuid = NULL; + VIR_AUTOFREE(char *) uuid = NULL; xmlNodePtr virtPortNode; xmlNodePtr vlanNode; xmlNodePtr bandwidthNode; xmlNodePtr addressNode; - char *trustGuestRxFilters = NULL; - char *mac = NULL; - char *macmgr = NULL; - char *mode = NULL; - char *plugtype = NULL; - char *managed = NULL; - char *driver = NULL; - char *class_id = NULL; + VIR_AUTOFREE(char *) trustGuestRxFilters = NULL; + VIR_AUTOFREE(char *) mac = NULL; + VIR_AUTOFREE(char *) macmgr = NULL; + VIR_AUTOFREE(char *) mode = NULL; + VIR_AUTOFREE(char *) plugtype = NULL; + VIR_AUTOFREE(char *) managed = NULL; + VIR_AUTOFREE(char *) driver = NULL; + VIR_AUTOFREE(char *) class_id = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -255,14 +255,6 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) } cleanup: - VIR_FREE(class_id); - VIR_FREE(uuid); - VIR_FREE(plugtype); - VIR_FREE(mac); - VIR_FREE(mode); - VIR_FREE(macmgr); - VIR_FREE(driver); - VIR_FREE(managed); return def; error: -- 2.21.0

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/virnetworkobj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 47c142998e..ed1f02f4c5 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1750,6 +1750,7 @@ virNetworkObjDeleteAllPorts(virNetworkObjPtr net, ret = 0; cleanup: + VIR_DIR_CLOSE(dh); return ret; } -- 2.21.0

On 6/18/19 1:39 PM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/virnetworkobj.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 47c142998e..ed1f02f4c5 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1750,6 +1750,7 @@ virNetworkObjDeleteAllPorts(virNetworkObjPtr net,
Just need to initialize dh = NULL John
ret = 0; cleanup: + VIR_DIR_CLOSE(dh); return ret; }

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/virnetworkobj.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index ed1f02f4c5..fd4bf779b9 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1810,10 +1810,12 @@ virNetworkObjPortListExport(virNetworkPtr net, }; int ret = -1; - *ports = NULL; + if (ports) { + *ports = NULL; - if (ports && VIR_ALLOC_N(data.ports, virHashSize(obj->ports) + 1) < 0) - goto cleanup; + if (VIR_ALLOC_N(data.ports, virHashSize(obj->ports) + 1) < 0) + goto cleanup; + } virHashForEach(obj->ports, virNetworkObjPortListExportCallback, &data); -- 2.21.0

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ee651e7742..c69d382d70 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30415,7 +30415,11 @@ virDomainNetDefToNetworkPort(virDomainDefPtr dom, if (VIR_ALLOC(port) < 0) return NULL; - virUUIDGenerate(port->uuid); + if (virUUIDGenerate(port->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate UUID")); + goto error; + } memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN); if (VIR_STRDUP(port->ownername, dom->name) < 0) @@ -30576,7 +30580,11 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, return NULL; /* Bad - we need to preserve original port uuid */ - virUUIDGenerate(port->uuid); + if (virUUIDGenerate(port->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate UUID")); + goto error; + } memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN); if (VIR_STRDUP(port->ownername, dom->name) < 0) -- 2.21.0

On 6/18/19 1:39 PM, Daniel P. Berrangé wrote:
Fixes for bugs spotted by coverity
Daniel P. Berrangé (4): conf: fix leak when parsing network port XML conf: fix leak of directory handle when loading network ports conf: fix NULL deref when exporting ports conf: add error checking of UUID generation
src/conf/domain_conf.c | 12 ++++++++++-- src/conf/virnetworkobj.c | 9 ++++++--- src/conf/virnetworkportdef.c | 26 +++++++++----------------- 3 files changed, 25 insertions(+), 22 deletions(-)
Fix the nit in 2 and that makes coverity happy again... Thanks Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
Daniel P. Berrangé
-
John Ferlan