aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorManos Pitsidianakis <el13635@mail.ntua.gr>2017-07-14 17:35:48 +0300
committerKevin Wolf <kwolf@redhat.com>2017-08-01 18:09:33 +0200
commit180ca19ae02be70f9b158bfd7dec1ff123b9cf8c (patch)
treea3272008113fdfc2eaa3eb723d6e481fcc2909f3
parent998cbd6a44cd96044f56713274bdf6a94cd721c0 (diff)
downloadqemu-arm-180ca19ae02be70f9b158bfd7dec1ff123b9cf8c.tar.gz
block: fix leaks in bdrv_open_driver()
bdrv_open_driver() is called in two places, bdrv_new_open_driver() and bdrv_open_common(). In the latter, failure cleanup in is in its caller, bdrv_open_inherit(), which unrefs the bs->file of the failed driver open if it exists. Let's move the bs->file cleanup to bdrv_open_driver() to take care of all callers and do not set bs->drv to NULL unless the driver's open function failed. When bs is destroyed by removing its last reference, it calls bdrv_close() which checks bs->drv to perform the needed cleanups and also call the driver's close function. Since it cleans up options and opaque we must take care not leave dangling pointers. The error paths in bdrv_open_driver() are now two: If open fails, drv->bdrv_close() should not be called. Unref the child if it exists, free what we allocated and set bs->drv to NULL. Return the error and let callers free their stuff. If open succeeds but we fail after, return the error and let callers unref and delete their bs, while cleaning up their allocations. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-rw-r--r--block.c22
1 files changed, 11 insertions, 11 deletions
diff --git a/block.c b/block.c
index 7a78bc647b..ce9cce7b3c 100644
--- a/block.c
+++ b/block.c
@@ -1119,20 +1119,19 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
} else {
error_setg_errno(errp, -ret, "Could not open image");
}
- goto free_and_fail;
+ goto open_failed;
}
ret = refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not refresh total sector count");
- goto free_and_fail;
+ return ret;
}
bdrv_refresh_limits(bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
- ret = -EINVAL;
- goto free_and_fail;
+ return -EINVAL;
}
assert(bdrv_opt_mem_align(bs) != 0);
@@ -1140,12 +1139,14 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
assert(is_power_of_2(bs->bl.request_alignment));
return 0;
-
-free_and_fail:
- /* FIXME Close bs first if already opened*/
+open_failed:
+ bs->drv = NULL;
+ if (bs->file != NULL) {
+ bdrv_unref_child(bs, bs->file);
+ bs->file = NULL;
+ }
g_free(bs->opaque);
bs->opaque = NULL;
- bs->drv = NULL;
return ret;
}
@@ -1166,7 +1167,9 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
ret = bdrv_open_driver(bs, drv, node_name, bs->options, flags, errp);
if (ret < 0) {
QDECREF(bs->explicit_options);
+ bs->explicit_options = NULL;
QDECREF(bs->options);
+ bs->options = NULL;
bdrv_unref(bs);
return NULL;
}
@@ -2600,9 +2603,6 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
fail:
blk_unref(file);
- if (bs->file != NULL) {
- bdrv_unref_child(bs, bs->file);
- }
QDECREF(snapshot_options);
QDECREF(bs->explicit_options);
QDECREF(bs->options);