mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

block: fix leaks in bdrv_open_driver()

Browse Source 
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>
This commit is contained in:
Manos Pitsidianakis 2017-07-14 17:35:48 +03:00 committed by Kevin Wolf
parent 998cbd6a44
commit 180ca19ae0
1 changed files with 11 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
block.c
View File

@ -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);