From d0b1ef237ccea4dc6701a25ab0efa12941ce2890 Mon Sep 17 00:00:00 2001 From: Jiabo Feng Date: Thu, 11 Jul 2024 14:38:44 +0800 Subject: [PATCH] QEMU update to version 4.1.0-85: - 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 --- ...names-only-when-explicitly-requested.patch | 223 +++++++++ ...ntroduce-bdrv_open_file_child-helper.patch | 459 ++++++++++++++++++ ...reopen-data_file-in-invalidate_cache.patch | 211 ++++++++ ...-data_file-with-BDRV_O_NO_IO-CVE-202.patch | 108 +++++ qemu.spec | 12 +- 5 files changed, 1012 insertions(+), 1 deletion(-) create mode 100644 block-Parse-filenames-only-when-explicitly-requested.patch create mode 100644 block-introduce-bdrv_open_file_child-helper.patch create mode 100644 qcow2-Do-not-reopen-data_file-in-invalidate_cache.patch create mode 100644 qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO-CVE-202.patch diff --git a/block-Parse-filenames-only-when-explicitly-requested.patch b/block-Parse-filenames-only-when-explicitly-requested.patch new file mode 100644 index 00000000..d9e50d66 --- /dev/null +++ b/block-Parse-filenames-only-when-explicitly-requested.patch @@ -0,0 +1,223 @@ +From 7619ae94b2d22fab824c1ea79e83b094d9e150f8 Mon Sep 17 00:00:00 2001 +From: liuxiangdong +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 +Reviewed-by: Eric Blake +Reviewed-by: Stefan Hajnoczi +Reviewed-by: Hanna Czenczek +Signed-off-by: liuxiangdong +--- + 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 + diff --git a/block-introduce-bdrv_open_file_child-helper.patch b/block-introduce-bdrv_open_file_child-helper.patch new file mode 100644 index 00000000..a0412510 --- /dev/null +++ b/block-introduce-bdrv_open_file_child-helper.patch @@ -0,0 +1,459 @@ +From 34058a6ad98adeca582ece7ad75b5705931101c0 Mon Sep 17 00:00:00 2001 +From: liuxiangdong +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 +Reviewed-by: Hanna Reitz +Message-Id: <20220726201134.924743-3-vsementsov@yandex-team.ru> +Reviewed-by: Kevin Wolf +Signed-off-by: Kevin Wolf +Signed-off-by: liuxiangdong +--- + 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 + diff --git a/qcow2-Do-not-reopen-data_file-in-invalidate_cache.patch b/qcow2-Do-not-reopen-data_file-in-invalidate_cache.patch new file mode 100644 index 00000000..276389f5 --- /dev/null +++ b/qcow2-Do-not-reopen-data_file-in-invalidate_cache.patch @@ -0,0 +1,211 @@ +From 013c6ad4645fc8411bb4615aa201b1f0b8a01003 Mon Sep 17 00:00:00 2001 +From: Hanna Reitz +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 +Message-Id: <20220427114057.36651-3-hreitz@redhat.com> +Reviewed-by: Eric Blake +Signed-off-by: Kevin Wolf +Signed-off-by: liuxiangdong +--- + 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 + diff --git a/qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO-CVE-202.patch b/qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO-CVE-202.patch new file mode 100644 index 00000000..4aea3b71 --- /dev/null +++ b/qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO-CVE-202.patch @@ -0,0 +1,108 @@ +From 89d32a19f14934bac61bb80955b8b29ac1ac9109 Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +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 +Reviewed-by: Eric Blake +Reviewed-by: Stefan Hajnoczi +Reviewed-by: Hanna Czenczek +--- + 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 + diff --git a/qemu.spec b/qemu.spec index 284f375b..bdea9377 100644 --- a/qemu.spec +++ b/qemu.spec @@ -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 +- 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 - 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) -- Gitee