Skip to content

Commit

Permalink
block: Parse filenames only when explicitly requested
Browse files Browse the repository at this point in the history 
When handling image filenames from legacy options such as -drive or from
tools, these filenames are parsed for protocol prefixes, including for
the json:{} pseudo-protocol.

This behaviour is intended for filenames that come directly from the
command line and for backing files, which may come from the image file
itself. Higher level management tools generally take care to verify that
untrusted images don't contain a bad (or any) backing file reference;
'qemu-img info' is a suitable tool for this.

However, for other files that can be referenced in images, such as
qcow2 data files or VMDK extents, the string from the image file is
usually not verified by management tools - and 'qemu-img info' wouldn't
be suitable because in contrast to backing files, it already opens these
other referenced files. So here the string should be interpreted as a
literal local filename. More complex configurations need to be specified
explicitly on the command line or in QMP.

This patch changes bdrv_open_inherit() so that it only parses filenames
if a new parameter parse_filename is true. It is set for the top level
in bdrv_open(), for the file child and for the backing file child. All
other callers pass false and disable filename parsing this way.

Cc: [email protected]
Signed-off-by: Kevin Wolf <[email protected]>
Reviewed-by: Eric Blake <[email protected]>
Reviewed-by: Stefan Hajnoczi <[email protected]>
Reviewed-by: Hanna Czenczek <[email protected]>
  • Loading branch information
@kevmw
kevmw committed Jul 2, 2024
1 parent 7e11106 commit 7ead946
Showing 1 changed file with 57 additions and 33 deletions.
90 changes: 57 additions & 33 deletions block.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
BlockDriverState *parent,
const BdrvChildClass *child_class,
BdrvChildRole child_role,
bool parse_filename,
Error **errp);

static bool bdrv_recurse_has_child(BlockDriverState *bs,
Expand Down Expand Up @@ -2055,7 +2056,8 @@ static void parse_json_protocol(QDict *options, const char **pfilename,
* block driver has been specified explicitly.
*/
static int bdrv_fill_options(QDict **options, const char *filename,
int *flags, Error **errp)
int *flags, bool allow_parse_filename,
Error **errp)
{
const char *drvname;
bool protocol = *flags & BDRV_O_PROTOCOL;
Expand Down Expand Up @@ -2097,7 +2099,7 @@ static int bdrv_fill_options(QDict **options, const char *filename,
if (protocol && filename) {
if (!qdict_haskey(*options, "filename")) {
qdict_put_str(*options, "filename", filename);
parse_filename = true;
parse_filename = allow_parse_filename;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at "
"the same time");
Expand Down Expand Up @@ -3660,7 +3662,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
}

backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0, bs,
&child_of_bds, bdrv_backing_role(bs), errp);
&child_of_bds, bdrv_backing_role(bs), true,
errp);
if (!backing_hd) {
bs->open_flags |= BDRV_O_NO_BACKING;
error_prepend(errp, "Could not open backing file: ");
Expand Down Expand Up @@ -3694,7 +3697,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
static BlockDriverState *
bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
BlockDriverState *parent, const BdrvChildClass *child_class,
BdrvChildRole child_role, bool allow_none, Error **errp)
BdrvChildRole child_role, bool allow_none,
bool parse_filename, Error **errp)
{
BlockDriverState *bs = NULL;
QDict *image_options;
Expand Down Expand Up @@ -3725,7 +3729,8 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
}

bs = bdrv_open_inherit(filename, reference, image_options, 0,
parent, child_class, child_role, errp);
parent, child_class, child_role, parse_filename,
errp);
if (!bs) {
goto done;
}
Expand All @@ -3735,6 +3740,33 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
return bs;
}

static BdrvChild *bdrv_open_child_common(const char *filename,
QDict *options, const char *bdref_key,
BlockDriverState *parent,
const BdrvChildClass *child_class,
BdrvChildRole child_role,
bool allow_none, bool parse_filename,
Error **errp)
{
BlockDriverState *bs;
BdrvChild *child;

GLOBAL_STATE_CODE();

bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_class,
child_role, allow_none, parse_filename, errp);
if (bs == NULL) {
return NULL;
}

bdrv_graph_wrlock();
child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
errp);
bdrv_graph_wrunlock();

return child;
}

/*
* Opens a disk image whose options are given as BlockdevRef in another block
* device's options.
Expand All @@ -3758,27 +3790,15 @@ BdrvChild *bdrv_open_child(const char *filename,
BdrvChildRole child_role,
bool allow_none, Error **errp)
{
BlockDriverState *bs;
BdrvChild *child;

GLOBAL_STATE_CODE();

bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_class,
child_role, allow_none, errp);
if (bs == NULL) {
return NULL;
}

bdrv_graph_wrlock();
child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
errp);
bdrv_graph_wrunlock();

return child;
return bdrv_open_child_common(filename, options, bdref_key, parent,
child_class, child_role, allow_none, false,
errp);
}

/*
* Wrapper on bdrv_open_child() for most popular case: open primary child of bs.
* This does mostly the same as bdrv_open_child(), but for opening the primary
* child of a node. A notable difference from bdrv_open_child() is that it
* enables filename parsing for protocol names (including json:).
*
* @parent can move to a different AioContext in this function.
*/
Expand All @@ -3793,8 +3813,8 @@ int bdrv_open_file_child(const char *filename,
role = parent->drv->is_filter ?
(BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY) : BDRV_CHILD_IMAGE;

if (!bdrv_open_child(filename, options, bdref_key, parent,
&child_of_bds, role, false, errp))
if (!bdrv_open_child_common(filename, options, bdref_key, parent,
&child_of_bds, role, false, true, errp))
{
return -EINVAL;
}
Expand Down Expand Up @@ -3839,7 +3859,8 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)

}

bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, 0, errp);
bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, 0, false,
errp);
obj = NULL;
qobject_unref(obj);
visit_free(v);
Expand Down Expand Up @@ -3929,7 +3950,7 @@ static BlockDriverState * no_coroutine_fn
bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
int flags, BlockDriverState *parent,
const BdrvChildClass *child_class, BdrvChildRole child_role,
Error **errp)
bool parse_filename, Error **errp)
{
int ret;
BlockBackend *file = NULL;
Expand Down Expand Up @@ -3977,9 +3998,11 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
}

/* json: syntax counts as explicit options, as if in the QDict */
parse_json_protocol(options, &filename, &local_err);
if (local_err) {
goto fail;
if (parse_filename) {
parse_json_protocol(options, &filename, &local_err);
if (local_err) {
goto fail;
}
}

bs->explicit_options = qdict_clone_shallow(options);
Expand All @@ -4004,7 +4027,8 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
parent->open_flags, parent->options);
}

ret = bdrv_fill_options(&options, filename, &flags, &local_err);
ret = bdrv_fill_options(&options, filename, &flags, parse_filename,
&local_err);
if (ret < 0) {
goto fail;
}
Expand Down Expand Up @@ -4073,7 +4097,7 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,

file_bs = bdrv_open_child_bs(filename, options, "file", bs,
&child_of_bds, BDRV_CHILD_IMAGE,
true, &local_err);
true, true, &local_err);
if (local_err) {
goto fail;
}
Expand Down Expand Up @@ -4222,7 +4246,7 @@ BlockDriverState *bdrv_open(const char *filename, const char *reference,
GLOBAL_STATE_CODE();

return bdrv_open_inherit(filename, reference, options, flags, NULL,
NULL, 0, errp);
NULL, 0, true, errp);
}

/* Return true if the NULL-terminated @list contains @str */
Expand Down

0 comments on commit 7ead946

Please sign in to comment.