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

QEMU update to version 4.1.0-85:

Browse Source 
- block: Parse filenames only when explicitly requested (CVE-2024-4467)
- block: introduce bdrv_open_file_child() helper
- qcow2: Don't open data_file with BDRV_O_NO_IO (CVE-2024-4467)
- qcow2: Do not reopen data_file in invalidate_cache

Signed-off-by: Jiabo Feng <fengjiabo1@huawei.com>
This commit is contained in:
Jiabo Feng 2024-07-11 14:38:44 +08:00
parent 1fe98a3a7b
commit d0b1ef237c
5 changed files with 1012 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

223
block-Parse-filenames-only-when-explicitly-requested.patch Normal file
View File

@ -0,0 +1,223 @@
From 7619ae94b2d22fab824c1ea79e83b094d9e150f8 Mon Sep 17 00:00:00 2001
From: liuxiangdong <liuxiangdong5@huawei.com>
Date: Wed, 19 Jun 2024 19:15:15 +0800
Subject: [PATCH] block: Parse filenames only when explicitly requested
(CVE-2024-4467)
cherry-pick from https://github.com/qemu/qemu/commit/7ead946998610657d38d1a505d5f25300d4ca613
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: qemu-stable@nongnu.org
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>
Signed-off-by: liuxiangdong <liuxiangdong5@huawei.com>
---
block.c | 69 +++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 45 insertions(+), 24 deletions(-)
diff --git a/block.c b/block.c
index 0e841dcf09..510857311e 100644
--- a/block.c
+++ b/block.c
@@ -76,6 +76,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
QDict *options, int flags,
BlockDriverState *parent,
const BdrvChildRole *child_role,
+ bool parse_filename,
Error **errp);
/* If non-zero, use only whitelisted block drivers */
@@ -1620,7 +1621,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;
@@ -1660,7 +1662,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");
@@ -2654,7 +2656,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
}
backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0, bs,
- &child_backing, errp);
+ &child_backing, true, errp);
if (!backing_hd) {
bs->open_flags |= BDRV_O_NO_BACKING;
error_prepend(errp, "Could not open backing file: ");
@@ -2689,7 +2691,7 @@ free_exit:
static BlockDriverState *
bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
BlockDriverState *parent, const BdrvChildRole *child_role,
- bool allow_none, Error **errp)
+ bool allow_none, bool parse_filename, Error **errp)
{
BlockDriverState *bs = NULL;
QDict *image_options;
@@ -2720,7 +2722,7 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
}
bs = bdrv_open_inherit(filename, reference, image_options, 0,
- parent, child_role, errp);
+ parent, child_role, parse_filename, errp);
if (!bs) {
goto done;
}
@@ -2730,6 +2732,24 @@ done:
return bs;
}
+static BdrvChild *bdrv_open_child_common(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent,
+ const BdrvChildRole *child_role,
+ bool allow_none, bool parse_filename,
+ Error **errp)
+{
+ BlockDriverState *bs;
+
+ bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_role,
+ allow_none, parse_filename, errp);
+ if (bs == NULL) {
+ return NULL;
+ }
+
+ return bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
+}
+
/*
* Opens a disk image whose options are given as BlockdevRef in another block
* device's options.
@@ -2750,26 +2770,22 @@ BdrvChild *bdrv_open_child(const char *filename,
const BdrvChildRole *child_role,
bool allow_none, Error **errp)
{
- BlockDriverState *bs;
-
- bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_role,
- allow_none, errp);
- if (bs == NULL) {
- return NULL;
- }
-
- return bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
+ return bdrv_open_child_common(filename, options, bdref_key, parent,
+ 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:).
*/
int bdrv_open_file_child(const char *filename,
QDict *options, const char *bdref_key,
BlockDriverState *parent, Error **errp)
{
- parent->file = bdrv_open_child(filename, options, bdref_key, parent,
- &child_file, false, errp);
+ parent->file = bdrv_open_child_common(filename, options, bdref_key, parent,
+ &child_file, false, true, errp);
return parent->file ? 0 : -EINVAL;
}
@@ -2812,7 +2828,8 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
}
- bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, errp);
+ bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, false,
+ errp);
obj = NULL;
fail:
@@ -2911,6 +2928,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
QDict *options, int flags,
BlockDriverState *parent,
const BdrvChildRole *child_role,
+ bool parse_filename,
Error **errp)
{
int ret;
@@ -2954,9 +2972,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);
@@ -2967,7 +2987,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 (local_err) {
goto fail;
}
@@ -3032,7 +3053,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
BlockDriverState *file_bs;
file_bs = bdrv_open_child_bs(filename, options, "file", bs,
- &child_file, true, &local_err);
+ &child_file, true, true, &local_err);
if (local_err) {
goto fail;
}
@@ -3177,7 +3198,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, errp);
+ NULL, true, errp);
}
/* Return true if the NULL-terminated @list contains @str */
--
2.41.0.windows.1

459
block-introduce-bdrv_open_file_child-helper.patch Normal file
View File

@ -0,0 +1,459 @@
From 34058a6ad98adeca582ece7ad75b5705931101c0 Mon Sep 17 00:00:00 2001
From: liuxiangdong <liuxiangdong5@huawei.com>
Date: Wed, 19 Jun 2024 19:09:26 +0800
Subject: [PATCH] block: introduce bdrv_open_file_child() helper
cherry-pick from: https://github.com/qemu/qemu/commit/83930780325b144a5908c45b3957b9b6457b3831
Almost all drivers call bdrv_open_child() similarly. Let's create a
helper for this.
The only not updated drivers that call bdrv_open_child() to set
bs->file are raw-format and snapshot-access:
raw-format sometimes want to have filtered child but
don't set drv->is_filter to true.
snapshot-access wants only DATA | PRIMARY
Possibly we should implement drv->is_filter_func() handler, to consider
raw-format as filter when it works as filter.. But it's another story.
Note also, that we decrease assignments to bs->file in code: it helps
us restrict modifying this field in further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220726201134.924743-3-vsementsov@yandex-team.ru>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: liuxiangdong <liuxiangdong5@huawei.com>
---
block.c | 13 +++++++++++++
block/blkdebug.c | 7 +++----
block/blklogwrites.c | 6 ++----
block/blkreplay.c | 6 ++----
block/blkverify.c | 7 +++----
block/bochs.c | 7 +++----
block/cloop.c | 7 +++----
block/copy-on-read.c | 8 ++++----
block/crypto.c | 11 ++++++-----
block/dmg.c | 7 +++----
block/parallels.c | 7 +++----
block/qcow.c | 6 ++----
block/qcow2.c | 8 ++++----
block/qed.c | 8 ++++----
block/replication.c | 7 +++----
block/throttle.c | 7 +++----
block/vdi.c | 7 +++----
block/vhdx.c | 7 +++----
block/vmdk.c | 7 +++----
block/vpc.c | 7 +++----
include/block/block.h | 3 +++
21 files changed, 76 insertions(+), 77 deletions(-)
diff --git a/block.c b/block.c
index 29e504b86a..0e841dcf09 100644
--- a/block.c
+++ b/block.c
@@ -2761,6 +2761,19 @@ BdrvChild *bdrv_open_child(const char *filename,
return bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
}
+/*
+ * Wrapper on bdrv_open_child() for most popular case: open primary child of bs.
+ */
+int bdrv_open_file_child(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent, Error **errp)
+{
+ parent->file = bdrv_open_child(filename, options, bdref_key, parent,
+ &child_file, false, errp);
+
+ return parent->file ? 0 : -EINVAL;
+}
+
/* TODO Future callers may need to specify parent/child_role in order for
* option inheritance to work. Existing callers use it for the root node. */
BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5ae96c52b0..15210da7dd 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -420,10 +420,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
s->state = 1;
/* Open the image file */
- bs->file = bdrv_open_child(qemu_opt_get(opts, "x-image"), options, "image",
- bs, &child_file, false, &local_err);
- if (local_err) {
- ret = -EINVAL;
+ ret = bdrv_open_file_child(qemu_opt_get(opts, "x-image"), options, "image",
+ bs, &local_err);
+ if (ret < 0) {
error_propagate(errp, local_err);
goto out;
}
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 04d8b33607..81149d432d 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -157,10 +157,8 @@ static int blk_log_writes_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Open the file */
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
- &local_err);
- if (local_err) {
- ret = -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, &local_err);
+ if (ret < 0) {
error_propagate(errp, local_err);
goto fail;
}
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 2b7931b940..e07d137f31 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -27,10 +27,8 @@ static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
/* Open the image file */
- bs->file = bdrv_open_child(NULL, options, "image",
- bs, &child_file, false, &local_err);
- if (local_err) {
- ret = -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "image", bs, &local_err);
+ if (ret < 0) {
error_propagate(errp, local_err);
goto fail;
}
diff --git a/block/blkverify.c b/block/blkverify.c
index 304b0a1368..65beeff3e5 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -124,10 +124,9 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Open the raw file */
- bs->file = bdrv_open_child(qemu_opt_get(opts, "x-raw"), options, "raw",
- bs, &child_file, false, &local_err);
- if (local_err) {
- ret = -EINVAL;
+ ret = bdrv_open_file_child(qemu_opt_get(opts, "x-raw"), options, "raw",
+ bs, &local_err);
+ if (ret < 0) {
error_propagate(errp, local_err);
goto fail;
}
diff --git a/block/bochs.c b/block/bochs.c
index 962f18592d..d8833a1b94 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -110,10 +110,9 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));
diff --git a/block/cloop.c b/block/cloop.c
index 384c9735bb..94cfbe93b4 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -71,10 +71,9 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
/* read header */
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 6631f30205..bc1d83128e 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -28,10 +28,10 @@
static int cor_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
- errp);
- if (!bs->file) {
- return -EINVAL;
+ int ret;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
diff --git a/block/crypto.c b/block/crypto.c
index 8237424ae6..bb92854cba 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -194,15 +194,14 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
BlockCrypto *crypto = bs->opaque;
QemuOpts *opts = NULL;
Error *local_err = NULL;
- int ret = -EINVAL;
+ int ret;
QCryptoBlockOpenOptions *open_opts = NULL;
unsigned int cflags = 0;
QDict *cryptoopts = NULL;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
bs->supported_write_flags = BDRV_REQ_FUA &
@@ -211,6 +210,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
opts = qemu_opts_create(opts_spec, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (local_err) {
+ ret = -EINVAL;
error_propagate(errp, local_err);
goto cleanup;
}
@@ -220,6 +220,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
open_opts = block_crypto_open_opts_init(cryptoopts, errp);
if (!open_opts) {
+ ret = -EINVAL;
goto cleanup;
}
diff --git a/block/dmg.c b/block/dmg.c
index 45f6b28f17..ad4141fbaa 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -439,10 +439,9 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
block_module_load_one("dmg-bz2");
diff --git a/block/parallels.c b/block/parallels.c
index 00fae125d1..721cbcb168 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -728,10 +728,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
Error *local_err = NULL;
char *buf;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
ret = bdrv_pread(bs->file, 0, &ph, sizeof(ph));
diff --git a/block/qcow.c b/block/qcow.c
index 5bdf72ba33..04776d102f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -130,10 +130,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
qdict_extract_subqdict(options, &encryptopts, "encrypt.");
encryptfmt = qdict_get_try_str(encryptopts, "format");
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- ret = -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index c1992c571a..e3331570b2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1775,11 +1775,11 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
.errp = errp,
.ret = -EINPROGRESS
};
+ int ret;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
/* Initialise locks */
diff --git a/block/qed.c b/block/qed.c
index 77c7cef175..6948b0c62f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -545,11 +545,11 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
.errp = errp,
.ret = -EINPROGRESS
};
+ int ret;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
bdrv_qed_init_state(bs);
diff --git a/block/replication.c b/block/replication.c
index 23b2993d74..52b40dfb22 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -90,10 +90,9 @@ static int replication_open(BlockDriverState *bs, QDict *options,
const char *mode;
const char *top_id;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
ret = -EINVAL;
diff --git a/block/throttle.c b/block/throttle.c
index 0349f42257..0b3dd23214 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -81,10 +81,9 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
char *group;
int ret;
- bs->file = bdrv_open_child(NULL, options, "file", bs,
- &child_file, false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
bs->supported_write_flags = bs->file->bs->supported_write_flags |
BDRV_REQ_WRITE_UNCHANGED;
diff --git a/block/vdi.c b/block/vdi.c
index b9845a4cbd..fa7d2f41f7 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -378,10 +378,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
Error *local_err = NULL;
QemuUUID uuid_link, uuid_parent;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
logout("\n");
diff --git a/block/vhdx.c b/block/vhdx.c
index d6070b6fa8..7a414de2e4 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -904,10 +904,9 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
uint64_t signature;
Error *local_err = NULL;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
s->bat = NULL;
diff --git a/block/vmdk.c b/block/vmdk.c
index bd36ece125..d0362b85f5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1243,10 +1243,9 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
uint32_t magic;
Error *local_err = NULL;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
buf = vmdk_read_desc(bs->file, 0, errp);
diff --git a/block/vpc.c b/block/vpc.c
index 3a88e28e2b..291172707b 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -228,10 +228,9 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
int64_t bs_size;
- bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
- false, errp);
- if (!bs->file) {
- return -EINVAL;
+ ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+ if (ret < 0) {
+ return ret;
}
opts = qemu_opts_create(&vpc_runtime_opts, NULL, 0, &error_abort);
diff --git a/include/block/block.h b/include/block/block.h
index 50a07c1c33..e07b0786e2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -303,6 +303,9 @@ BdrvChild *bdrv_open_child(const char *filename,
BlockDriverState* parent,
const BdrvChildRole *child_role,
bool allow_none, Error **errp);
+int bdrv_open_file_child(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent, Error **errp);
BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
Error **errp);
--
2.41.0.windows.1

211
qcow2-Do-not-reopen-data_file-in-invalidate_cache.patch Normal file
View File

@ -0,0 +1,211 @@
From 013c6ad4645fc8411bb4615aa201b1f0b8a01003 Mon Sep 17 00:00:00 2001
From: Hanna Reitz <hreitz@redhat.com>
Date: Wed, 27 Apr 2022 13:40:55 +0200
Subject: [PATCH] qcow2: Do not reopen data_file in invalidate_cache
qcow2_co_invalidate_cache() closes and opens the qcow2 file, by calling
qcow2_close() and qcow2_do_open(). These two functions must thus be
usable from both a global-state and an I/O context.
As they are, they are not safe to call in an I/O context, because they
use bdrv_unref_child() and bdrv_open_child() to close/open the data_file
child, respectively, both of which are global-state functions. When
used from qcow2_co_invalidate_cache(), we do not need to close/open the
data_file child, though (we do not do this for bs->file or bs->backing
either), and so we should skip it in the qcow2_co_invalidate_cache()
path.
To do so, add a parameter to qcow2_do_open() and qcow2_close() to make
them skip handling s->data_file, and have qcow2_co_invalidate_cache()
exempt it from the memset() on the BDRVQcow2State.
(Note that the QED driver similarly closes/opens the QED image by
invoking bdrv_qed_close()+bdrv_qed_do_open(), but both functions seem
safe to use in an I/O context.)
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/945
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220427114057.36651-3-hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: liuxiangdong <liuxiangdong5@huawei.com>
---
block/qcow2.c | 96 +++++++++++++++++++++++++++++++--------------------
1 file changed, 58 insertions(+), 38 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 1909df6e1d..8e1b5ffe2d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1204,7 +1204,8 @@ static int qcow2_update_options(BlockDriverState *bs, QDict *options,
/* Called with s->lock held. */
static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
- int flags, Error **errp)
+ int flags, bool open_data_file,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
unsigned int len, i;
@@ -1491,44 +1492,46 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
goto fail;
}
- /* Open external data file */
- s->data_file = bdrv_open_child(NULL, options, "data-file", bs, &child_file,
- true, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- ret = -EINVAL;
- goto fail;
- }
+ if (open_data_file) {
+ /* Open external data file */
+ s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
+ &child_file, true, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto fail;
+ }
- if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
- if (!s->data_file && s->image_data_file) {
- s->data_file = bdrv_open_child(s->image_data_file, options,
- "data-file", bs, &child_file,
- false, errp);
+ if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
+ if (!s->data_file && s->image_data_file) {
+ s->data_file = bdrv_open_child(s->image_data_file, options,
+ "data-file", bs, &child_file,
+ false, errp);
+ if (!s->data_file) {
+ ret = -EINVAL;
+ goto fail;
+ }
+ }
if (!s->data_file) {
+ error_setg(errp, "'data-file' is required for this image");
+ ret = -EINVAL;
+ goto fail;
+ }
+ } else {
+ if (s->data_file) {
+ error_setg(errp, "'data-file' can only be set for images with "
+ "an external data file");
ret = -EINVAL;
goto fail;
}
- }
- if (!s->data_file) {
- error_setg(errp, "'data-file' is required for this image");
- ret = -EINVAL;
- goto fail;
- }
- } else {
- if (s->data_file) {
- error_setg(errp, "'data-file' can only be set for images with an "
- "external data file");
- ret = -EINVAL;
- goto fail;
- }
- s->data_file = bs->file;
+ s->data_file = bs->file;
- if (data_file_is_raw(bs)) {
- error_setg(errp, "data-file-raw requires a data file");
- ret = -EINVAL;
- goto fail;
+ if (data_file_is_raw(bs)) {
+ error_setg(errp, "data-file-raw requires a data file");
+ ret = -EINVAL;
+ goto fail;
+ }
}
}
@@ -1705,7 +1708,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
fail:
g_free(s->image_data_file);
- if (has_data_file(bs)) {
+ if (open_data_file && has_data_file(bs)) {
bdrv_unref_child(bs, s->data_file);
}
g_free(s->unknown_header_fields);
@@ -1741,7 +1744,8 @@ static void coroutine_fn qcow2_open_entry(void *opaque)
BDRVQcow2State *s = qoc->bs->opaque;
qemu_co_mutex_lock(&s->lock);
- qoc->ret = qcow2_do_open(qoc->bs, qoc->options, qoc->flags, qoc->errp);
+ qoc->ret = qcow2_do_open(qoc->bs, qoc->options, qoc->flags, true,
+ qoc->errp);
qemu_co_mutex_unlock(&s->lock);
}
@@ -2391,7 +2395,7 @@ static int qcow2_inactivate(BlockDriverState *bs)
return result;
}
-static void qcow2_close(BlockDriverState *bs)
+static void qcow2_do_close(BlockDriverState *bs, bool close_data_file)
{
BDRVQcow2State *s = bs->opaque;
qemu_vfree(s->l1_table);
@@ -2416,7 +2420,7 @@ static void qcow2_close(BlockDriverState *bs)
g_free(s->image_backing_file);
g_free(s->image_backing_format);
- if (has_data_file(bs)) {
+ if (close_data_file && has_data_file(bs)) {
bdrv_unref_child(bs, s->data_file);
}
@@ -2424,10 +2428,16 @@ static void qcow2_close(BlockDriverState *bs)
qcow2_free_snapshots(bs);
}
+static void qcow2_close(BlockDriverState *bs)
+{
+ qcow2_do_close(bs, true);
+}
+
static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
Error **errp)
{
BDRVQcow2State *s = bs->opaque;
+ BdrvChild *data_file;
int flags = s->flags;
QCryptoBlock *crypto = NULL;
QDict *options;
@@ -2442,14 +2452,24 @@ static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
crypto = s->crypto;
s->crypto = NULL;
- qcow2_close(bs);
+ /*
+ * Do not reopen s->data_file (i.e., have qcow2_do_close() not close it,
+ * and then prevent qcow2_do_open() from opening it), because this function
+ * runs in the I/O path and as such we must not invoke global-state
+ * functions like bdrv_unref_child() and bdrv_open_child().
+ */
+ qcow2_do_close(bs, false);
+
+ data_file = s->data_file;
memset(s, 0, sizeof(BDRVQcow2State));
+ s->data_file = data_file;
+
options = qdict_clone_shallow(bs->options);
flags &= ~BDRV_O_INACTIVE;
qemu_co_mutex_lock(&s->lock);
- ret = qcow2_do_open(bs, options, flags, &local_err);
+ ret = qcow2_do_open(bs, options, flags, false, &local_err);
qemu_co_mutex_unlock(&s->lock);
qobject_unref(options);
if (local_err) {
--
2.41.0.windows.1

108
qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO-CVE-202.patch Normal file
View File

@ -0,0 +1,108 @@
From 89d32a19f14934bac61bb80955b8b29ac1ac9109 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Thu, 11 Apr 2024 15:06:01 +0200
Subject: [PATCH] qcow2: Don't open data_file with BDRV_O_NO_IO (CVE-2024-4467)
One use case for 'qemu-img info' is verifying that untrusted images
don't reference an unwanted external file, be it as a backing file or an
external data file. To make sure that calling 'qemu-img info' can't
already have undesired side effects with a malicious image, just don't
open the data file at all with BDRV_O_NO_IO. If nothing ever tries to do
I/O, we don't need to have it open.
This changes the output of iotests case 061, which used 'qemu-img info'
to show that opening an image with an invalid data file fails. After
this patch, it succeeds. Replace this part of the test with a qemu-io
call, but keep the final 'qemu-img info' to show that the invalid data
file is correctly displayed in the output.
Fixes: CVE-2024-4467
Cc: qemu-stable@nongnu.org
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>
---
block/qcow2.c | 17 ++++++++++++++++-
tests/qemu-iotests/061 | 6 ++++--
tests/qemu-iotests/061.out | 8 ++++++--
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 8e1b5ffe2d..c1992c571a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1492,7 +1492,22 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
goto fail;
}
- if (open_data_file) {
+ if (open_data_file && (flags & BDRV_O_NO_IO)) {
+ /*
+ * Don't open the data file for 'qemu-img info' so that it can be used
+ * to verify that an untrusted qcow2 image doesn't refer to external
+ * files.
+ *
+ * Note: This still makes has_data_file() return true.
+ */
+ if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
+ s->data_file = NULL;
+ } else {
+ s->data_file = bs->file;
+ }
+ qdict_extract_subqdict(options, NULL, "data-file.");
+ qdict_del(options, "data-file");
+ } else if (open_data_file) {
/* Open external data file */
s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
&child_file, true, &local_err);
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index d7dbd7e2c7..ff8ca908d0 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -268,12 +268,14 @@ $QEMU_IMG amend -o "data_file=foo" "$TEST_IMG"
echo
IMGOPTS="compat=1.1,data_file=$TEST_IMG.data" _make_test_img 64M
$QEMU_IMG amend -o "data_file=foo" "$TEST_IMG"
-_img_info --format-specific
+$QEMU_IO -c "read 0 4k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
+$QEMU_IO -c "open -o data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" -c "read 0 4k" | _filter_qemu_io
TEST_IMG="data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" _img_info --format-specific --image-opts
echo
$QEMU_IMG amend -o "data_file=" --image-opts "data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG"
-_img_info --format-specific
+$QEMU_IO -c "read 0 4k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
+$QEMU_IO -c "open -o data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" -c "read 0 4k" | _filter_qemu_io
TEST_IMG="data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" _img_info --format-specific --image-opts
echo
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 1aa7d37ff9..8c6f1dbde3 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -512,7 +512,9 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
qemu-img: data-file can only be set for images that use an external data file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 data_file=TEST_DIR/t.IMGFMT.data
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open 'foo': No such file or directory
+qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open 'foo': No such file or directory
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
image: TEST_DIR/t.IMGFMT
file format: IMGFMT
virtual size: 64 MiB (67108864 bytes)
@@ -525,7 +527,9 @@ Format specific information:
data file raw: false
corrupt: false
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'data-file' is required for this image
+qemu-io: can't open device TEST_DIR/t.IMGFMT: 'data-file' is required for this image
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
image: TEST_DIR/t.IMGFMT
file format: IMGFMT
virtual size: 64 MiB (67108864 bytes)
--
2.41.0.windows.1

12
qemu.spec
View File

@ -1,6 +1,6 @@
Name: qemu
Version: 4.1.0
Release: 84
Release: 85
Epoch: 10
Summary: QEMU is a generic and open source machine emulator and virtualizer
License: GPLv2 and BSD and MIT and CC-BY-SA-4.0
@ -411,6 +411,10 @@ Patch0398: hw-char-virtio-serial-bus-Protect-from-DMA-re-entran.patch
Patch0399: hw-virtio-virtio-crypto-Protect-from-DMA-re-entrancy.patch
Patch0400: hw-ide-reset-cancel-async-DMA-operation-before-reset.patch
Patch0401: tests-qtest-ahci-test-add-test-exposing-reset-issue-.patch
Patch0402: qcow2-Do-not-reopen-data_file-in-invalidate_cache.patch
Patch0403: qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO-CVE-202.patch
Patch0404: block-introduce-bdrv_open_file_child-helper.patch
Patch0405: block-Parse-filenames-only-when-explicitly-requested.patch
BuildRequires: flex
BuildRequires: bison
@ -811,6 +815,12 @@ getent passwd qemu >/dev/null || \
%endif
%changelog
* Thu Jul 11 2024 Jiabo Feng <fengjiabo1@huawei.com>
- block: Parse filenames only when explicitly requested (CVE-2024-4467)
- block: introduce bdrv_open_file_child() helper
- qcow2: Don't open data_file with BDRV_O_NO_IO (CVE-2024-4467)
- qcow2: Do not reopen data_file in invalidate_cache
* Sat Jun 15 2024 Jiabo Feng <fengjiabo1@huawei.com>
- tests/qtest: ahci-test: add test exposing reset issue with pending callback (Fix CVE-2023-5088)
- hw/ide: reset: cancel async DMA operation before resetting state (Fix CVE-2023-5088)