210 lines
8.4 KiB
Diff
210 lines
8.4 KiB
Diff
From 5cdbc87ab24a8cc4cf926158ec429d43d8a45f15 Mon Sep 17 00:00:00 2001
|
||
From: Jon Maloy <jmaloy@redhat.com>
|
||
Date: Wed, 5 Jun 2024 19:56:51 -0400
|
||
Subject: [PATCH 1/5] qcow2: Don't open data_file with BDRV_O_NO_IO
|
||
MIME-Version: 1.0
|
||
Content-Type: text/plain; charset=UTF-8
|
||
Content-Transfer-Encoding: 8bit
|
||
|
||
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: [1/5] 2e72d21c14d86645cf68eec78f49d5cc5d77581f
|
||
|
||
Conflicts: qcow2_do_open(): missing boolean ´open_data_file'.
|
||
We assume it to be true.
|
||
|
||
commit f9843ce5c519901654a7d8ba43ee95ce25ca13c2
|
||
Author: Kevin Wolf <kwolf@redhat.com>
|
||
Date: Thu Apr 11 15:06:01 2024 +0200
|
||
|
||
qcow2: Don't open data_file with BDRV_O_NO_IO
|
||
|
||
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.
|
||
|
||
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/qcow2.c | 87 +++++++++++++++++++++++---------------
|
||
tests/qemu-iotests/061 | 6 ++-
|
||
tests/qemu-iotests/061.out | 8 +++-
|
||
3 files changed, 62 insertions(+), 39 deletions(-)
|
||
|
||
diff --git a/block/qcow2.c b/block/qcow2.c
|
||
index d509016756..6ee1919612 100644
|
||
--- a/block/qcow2.c
|
||
+++ b/block/qcow2.c
|
||
@@ -1613,50 +1613,67 @@ 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_of_bds, BDRV_CHILD_DATA,
|
||
- true, errp);
|
||
- if (*errp) {
|
||
- ret = -EINVAL;
|
||
- goto fail;
|
||
- }
|
||
+ if (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 {
|
||
+ /* Open external data file */
|
||
+ s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
|
||
+ &child_of_bds, BDRV_CHILD_DATA,
|
||
+ true, errp);
|
||
+ if (*errp) {
|
||
+ 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_of_bds,
|
||
- BDRV_CHILD_DATA, 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_of_bds,
|
||
+ BDRV_CHILD_DATA, 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;
|
||
}
|
||
- }
|
||
- if (!s->data_file) {
|
||
- error_setg(errp, "'data-file' is required for this image");
|
||
- ret = -EINVAL;
|
||
- goto fail;
|
||
- }
|
||
|
||
- /* No data here */
|
||
- bs->file->role &= ~BDRV_CHILD_DATA;
|
||
+ /* No data here */
|
||
+ bs->file->role &= ~BDRV_CHILD_DATA;
|
||
|
||
- /* Must succeed because we have given up permissions if anything */
|
||
- bdrv_child_refresh_perms(bs, bs->file, &error_abort);
|
||
- } 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;
|
||
- }
|
||
+ /* Must succeed because we have given up permissions if anything */
|
||
+ bdrv_child_refresh_perms(bs, bs->file, &error_abort);
|
||
+ } 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;
|
||
+ }
|
||
}
|
||
}
|
||
|
||
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
|
||
index 9507c223bd..6a5bd47efc 100755
|
||
--- a/tests/qemu-iotests/061
|
||
+++ b/tests/qemu-iotests/061
|
||
@@ -322,12 +322,14 @@ $QEMU_IMG amend -o "data_file=foo" "$TEST_IMG"
|
||
echo
|
||
_make_test_img -o "compat=1.1,data_file=$TEST_IMG.data" 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 7ecbd4dea8..99b2307a23 100644
|
||
--- a/tests/qemu-iotests/061.out
|
||
+++ b/tests/qemu-iotests/061.out
|
||
@@ -545,7 +545,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)
|
||
@@ -560,7 +562,9 @@ Format specific information:
|
||
corrupt: false
|
||
extended l2: 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.39.3
|
||
|