From 156ac94463b42b0b9beea263af9866dfcd3683e0 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Tue, 19 Feb 2019 16:34:40 +0000 Subject: [PATCH] xen-block: stop leaking memory in xen_block_drive_create() The locally allocated QDict-s need to be freed. ('file_layer' will be freed implicitly since it is added as an object to 'driver_layer'). Spotted by Coverity: CID 1398649 While in the neighbourhood free 'driver' and 'filename' as soon as they are added to the QDicts. Freeing after the 'done' label doesn't make that much sense as, if the error path jumps to that label, the values would be NULL anyway. This patch also makes that more obvious by taking the error path if 'params' is NULL and then asserting that both driver and filename are non-NULL in the normal path. Reported-by: Peter Maydell Signed-off-by: Paul Durrant Message-Id: <20190219163440.15702-1-paul.durrant@citrix.com> Acked-by: Anthony PERARD Signed-off-by: Anthony PERARD --- hw/block/xen-block.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c index 37a456c207..70fc2455e8 100644 --- a/hw/block/xen-block.c +++ b/hw/block/xen-block.c @@ -743,12 +743,12 @@ static XenBlockDrive *xen_block_drive_create(const char *id, } g_strfreev(v); - } - - if (!filename) { - error_setg(errp, "no filename"); + } else { + error_setg(errp, "no params"); goto done; } + + assert(filename); assert(driver); drive = g_new0(XenBlockDrive, 1); @@ -758,6 +758,7 @@ static XenBlockDrive *xen_block_drive_create(const char *id, qdict_put_str(file_layer, "driver", "file"); qdict_put_str(file_layer, "filename", filename); + g_free(filename); if (mode && *mode != 'w') { qdict_put_bool(file_layer, "read-only", true); @@ -793,16 +794,17 @@ static XenBlockDrive *xen_block_drive_create(const char *id, driver_layer = qdict_new(); qdict_put_str(driver_layer, "driver", driver); + g_free(driver); + qdict_put_obj(driver_layer, "file", QOBJECT(file_layer)); g_assert(!drive->node_name); drive->node_name = xen_block_blockdev_add(drive->id, driver_layer, &local_err); -done: - g_free(driver); - g_free(filename); + qobject_unref(driver_layer); +done: if (local_err) { error_propagate(errp, local_err); xen_block_drive_destroy(drive, NULL); -- 2.11.4.GIT