Re: [libvirt] [PATCH] storage: Avoid leak in virStorageUtilGlusterExtractPoolSources()
On 04/05/2017 12:28 PM, Andrea Bolognani wrote:
The contents of volname would be leaked if the function were
to be passed an invalid pooltype by the caller.
Make sure the memory is released instead.
---
Initializing volname to NULL is strictly speaking not
necessary, but I like it better that way :)
src/storage/storage_util.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
This could be "as easy as" just adding a VIR_FREE(volname) in the "}
else {" case prior to virReportError and goto cleanup;
If you went with this option, then VIR_STEAL_PTR(src->dir, volname); and
VIR_STEAL_PTR(src->name, volname); instead of inlining.
And as some would say the comment is "obvious".
IDC which way it happens...
John
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 7cc125a..e0f948b 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2846,7 +2846,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
xmlXPathContextPtr ctxt = NULL;
xmlNodePtr *nodes = NULL;
virStoragePoolSource *src = NULL;
- char *volname;
+ char *volname = NULL;
size_t i;
int nnodes;
int ret = -1;
@@ -2871,12 +2871,17 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
if (pooltype == VIR_STORAGE_POOL_NETFS) {
src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;
+
+ /* Ownership of volname is passed to src */
src->dir = volname;
+ volname = NULL;
} else if (pooltype == VIR_STORAGE_POOL_GLUSTER) {
- src->name = volname;
-
if (VIR_STRDUP(src->dir, "/") < 0)
goto cleanup;
+
+ /* Ownership of volname is passed to src */
+ src->name = volname;
+ volname = NULL;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("unsupported gluster lookup"));
@@ -2894,6 +2899,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
ret = nnodes;
cleanup:
+ VIR_FREE(volname);
VIR_FREE(nodes);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(doc);