You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
261 lines
11 KiB
261 lines
11 KiB
From c4ba1f1755031a0ac2f600ed8c17e7dcb6b2b857 Mon Sep 17 00:00:00 2001
|
|
From: Jon Maloy <jmaloy@redhat.com>
|
|
Date: Wed, 5 Jun 2024 19:56:51 -0400
|
|
Subject: [PATCH 5/5] block: Parse filenames only when explicitly requested
|
|
|
|
RH-Author: Jon Maloy <jmaloy@redhat.com>
|
|
RH-MergeRequest: 5: EMBARGOED CVE-2024-4467 for rhel-8.10.z (PRDSC)
|
|
RH-Jira: RHEL-35616
|
|
RH-CVE: CVE-2024-4467
|
|
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
|
|
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
RH-Commit: [5/5] a3e197add64fc6950c4ac576e34d833dfae7ee34
|
|
|
|
Conflicts: - brdv_open_child_common(): bdrv_graph_wrlock/unlock()
|
|
don't exist in this code version. We ignore them.
|
|
bdrv_open_inherit(): no_coroutine_fn/GRAPH_UNLOCKED
|
|
doesn't exist. We ignore it.
|
|
- Changes to bdrv_open_file_child() didn't apply cleanly,
|
|
but fixing it is straight-forward.
|
|
- GLOBAL_STATE_CODE() not present in this code. Ignoring it.
|
|
- bdrv_open_file_child(): Need to continue setting of
|
|
parent->file.
|
|
|
|
commit f44c2941d4419e60f16dea3e9adca164e75aa78d
|
|
Author: Kevin Wolf <kwolf@redhat.com>
|
|
Date: Thu Apr 25 14:56:02 2024 +0200
|
|
|
|
block: Parse filenames only when explicitly requested
|
|
|
|
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.
|
|
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
Reviewed-by: Eric Blake <eblake@redhat.com>
|
|
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
|
|
Upstream: N/A, embargoed
|
|
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
|
|
|
|
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
|
|
---
|
|
block.c | 81 +++++++++++++++++++++++++++++++++++++++------------------
|
|
1 file changed, 56 insertions(+), 25 deletions(-)
|
|
|
|
diff --git a/block.c b/block.c
|
|
index 889f878565..ddebf50efa 100644
|
|
--- a/block.c
|
|
+++ b/block.c
|
|
@@ -82,6 +82,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,
|
|
@@ -1926,7 +1927,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;
|
|
@@ -1966,7 +1968,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");
|
|
@@ -3439,7 +3441,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: ");
|
|
@@ -3472,7 +3475,8 @@ free_exit:
|
|
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;
|
|
@@ -3503,7 +3507,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;
|
|
}
|
|
@@ -3513,6 +3518,29 @@ done:
|
|
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;
|
|
+
|
|
+ bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_class,
|
|
+ child_role, allow_none, parse_filename, errp);
|
|
+ if (bs == NULL) {
|
|
+ return NULL;
|
|
+ }
|
|
+
|
|
+ child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
|
|
+ errp);
|
|
+
|
|
+ return child;
|
|
+}
|
|
+
|
|
/*
|
|
* Opens a disk image whose options are given as BlockdevRef in another block
|
|
* device's options.
|
|
@@ -3534,20 +3562,17 @@ BdrvChild *bdrv_open_child(const char *filename,
|
|
BdrvChildRole child_role,
|
|
bool allow_none, Error **errp)
|
|
{
|
|
- BlockDriverState *bs;
|
|
-
|
|
- bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_class,
|
|
- child_role, allow_none, errp);
|
|
- if (bs == NULL) {
|
|
- return NULL;
|
|
- }
|
|
-
|
|
- return bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
|
|
- errp);
|
|
+ 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.
|
|
*/
|
|
int bdrv_open_file_child(const char *filename,
|
|
QDict *options, const char *bdref_key,
|
|
@@ -3558,8 +3583,9 @@ int bdrv_open_file_child(const char *filename,
|
|
role = parent->drv->is_filter ?
|
|
(BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY) : BDRV_CHILD_IMAGE;
|
|
|
|
- parent->file = bdrv_open_child(filename, options, bdref_key, parent,
|
|
- &child_of_bds, role, false, errp);
|
|
+ parent->file = bdrv_open_child_common(filename, options, bdref_key, parent,
|
|
+ &child_of_bds, role, false, true,
|
|
+ errp);
|
|
|
|
return parent->file ? 0 : -EINVAL;
|
|
}
|
|
@@ -3599,7 +3625,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);
|
|
@@ -3690,6 +3717,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
|
|
BlockDriverState *parent,
|
|
const BdrvChildClass *child_class,
|
|
BdrvChildRole child_role,
|
|
+ bool parse_filename,
|
|
Error **errp)
|
|
{
|
|
int ret;
|
|
@@ -3733,9 +3761,11 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
|
|
}
|
|
|
|
/* 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);
|
|
@@ -3760,7 +3790,8 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
|
|
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;
|
|
}
|
|
@@ -3829,7 +3860,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
|
|
|
|
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;
|
|
}
|
|
@@ -3974,7 +4005,7 @@ BlockDriverState *bdrv_open(const char *filename, const char *reference,
|
|
QDict *options, int flags, Error **errp)
|
|
{
|
|
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 */
|
|
--
|
|
2.39.3
|
|
|