[PATCH 0/2] storage_file: Fix probing of backing images with 'data_file' with auto-probed format
Patch 1 adds an counter-example test and patch 2 then fixes the bug. Peter Krempa (2): virstoragetest: Add example of a qcow2 image with a backing image pointing to a data_file without 'backing_format' virStorageSourceGetMetadataRecurse: Fix format probing exception for images with 'data_file' src/storage_file/storage_source.c | 3 ++- tests/virstoragetest.c | 7 ++++++- .../images/qcow2_datafile-auto.qcow2 | Bin 0 -> 196616 bytes 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 tests/virstoragetestdata/images/qcow2_datafile-auto.qcow2 -- 2.55.0
From: Peter Krempa <pkrempa@redhat.com> Add an image example which specifies a 'backing_file' without 'backing_format' and the backing file has 'data_file'. This was obtained by running a hacked qemu-img with bypassed requirement to specify backing file format: $ qemu-img create -f qcow2 -b datafile.qcow2 -u qcow2_datafile-auto.qcow2 1024 In the test directory. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 7 +++- .../images/qcow2_datafile-auto.qcow2 | Bin 0 -> 196616 bytes .../qcow2datafile-qcow2_qcow2-datafile-auto | 31 ++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/virstoragetestdata/images/qcow2_datafile-auto.qcow2 create mode 100644 tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index e568bd3141..1514991c20 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -488,13 +488,18 @@ mymain(void) /* qcow2 + datafile * 1) qcow2 image with data file - * 2) qcow2 -> qcow2 backing image with data file */ + * 2) qcow2 -> qcow2 backing image with data file + * 3) qcow2 (with no backing file format specified) -> qcow2 backing image with data file + */ TEST_CHAIN("qcow2-datafile", abs_srcdir "/virstoragetestdata/images/datafile.qcow2", VIR_STORAGE_FILE_QCOW2, EXP_PASS); TEST_CHAIN("qcow2datafile-qcow2_qcow2-datafile", abs_srcdir "/virstoragetestdata/images/qcow2datafile-datafile.qcow2", VIR_STORAGE_FILE_QCOW2, EXP_PASS); + TEST_CHAIN("qcow2datafile-qcow2_qcow2-datafile-auto", + abs_srcdir "/virstoragetestdata/images/qcow2_datafile-auto.qcow2", + VIR_STORAGE_FILE_QCOW2, EXP_PASS); /* broken qcow2 with a 'data_file' which is an empty string */ TEST_CHAIN("qcow2-datafile-broken", diff --git a/tests/virstoragetestdata/images/qcow2_datafile-auto.qcow2 b/tests/virstoragetestdata/images/qcow2_datafile-auto.qcow2 new file mode 100644 index 0000000000000000000000000000000000000000..e8a6c2710b5841236b8360d260b751f60349e2fa GIT binary patch literal 196616 zcmeIuyH3L}7zWTkX=lVE<Ovx15J*TYOw4JUib!n|Qdgx+JT?*wN-qOT%HDG<`TE(G z{qp$q6A_a*%7^+cBJNM)>8Xjxj-7MrJUY!?HyqMkVsDf0*NE&ZVp;d;P|WLeSsIru z+P>d)>AySj>LXQsQ?83;naW~SudC4uCX05{_0@J;w@r}_-FP};I-K7ut7Y*#E2<{- zb+sM2Ad7YRc_{j7wP<%u8jBv~F|X66?5+mS@~kiS#h*7{mPQSqGyKiN`=Z^?Mxy`# z00000000000000000000000000000000000000000000000000000000000000000 z0000000000z%9sc;gJ9U000000000000000000000000000000000000000000000 z0000000000000000000000000000000F27c1poj50000000000000000000000000 u00000000000000000000000000000000000000000000000000BH|Zt@-@W( literal 0 HcmV?d00001 diff --git a/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto b/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto new file mode 100644 index 0000000000..421e0c8d5a --- /dev/null +++ b/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto @@ -0,0 +1,31 @@ +path:ABS_SRCDIR/virstoragetestdata/images/qcow2_datafile-auto.qcow2 +backingStoreRaw: datafile.qcow2 +backingStoreRawFormat: <null>(-1) +dataFileRaw: <null> +capacity: 1024 +encryption: 0 +relPath:<null> +type:file +format:qcow2 +protocol:none +hostname:<null> + +path:ABS_SRCDIR/virstoragetestdata/images/datafile.qcow2 +backingStoreRaw: <null> +backingStoreRawFormat: none(0) +dataFileRaw: raw +capacity: 1024 +encryption: 0 +relPath:datafile.qcow2 +type:file +format:qcow2 +protocol:none +hostname:<null> + + dataFileStoreSource for 'ABS_SRCDIR/virstoragetestdata/images/datafile.qcow2': + path: ABS_SRCDIR/virstoragetestdata/images/raw + capacity: 0 + encryption: 0 + type:file + format:raw + -- 2.55.0
From: Peter Krempa <pkrempa@redhat.com> The image metadata crawler code generally forbids qcow2 images which have a 'backing_file' but don't specify a 'backing_file_fmt' header as we don't want to probe the format due to security implications. There's one notable exception for the last image in the chain which can be format probed if it doesn't have another 'backing_file'. As the comment in 'virStorageSourceGetMetadataRecurse' suggests we don't want to allow the probe if anything probed would influence access to more resources. Unfortunately that didn't involve 'data_file' header which gives more access. Fortunately there is no way for a guest OS having a 'raw' disk and access to libvirt's snapshot API to abuse this as libvirt's snapshot API does specify the header. Other cases where an arbitrary image is passed to libvirt can 'backing_file'+'backing_file_fmt' directly to access arbitrary file on disk so this bug is no worse in this regard. Nevertheless it's a bug which should be fixed. Closes: https://gitlab.com/libvirt/libvirt/-/work_items/904 Fixes: 0a3d177d9bd6cc608cc8e5769188e5f45f70ee62 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage_file/storage_source.c | 3 +- tests/virstoragetest.c | 2 +- .../qcow2datafile-qcow2_qcow2-datafile-auto | 31 ------------------- 3 files changed, 3 insertions(+), 33 deletions(-) delete mode 100644 tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c index e886433bb4..aa7e834819 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -1426,9 +1426,10 @@ virStorageSourceGetMetadataRecurse(virStorageSource *src, /* If we probed the format we MUST ensure that nothing else than the current * image is considered for security labelling and/or recursion. */ if (orig_format == VIR_STORAGE_FILE_AUTO) { - if (src->backingStoreRaw) { + if (src->backingStoreRaw || src->dataFileRaw) { src->format = VIR_STORAGE_FILE_RAW; VIR_FREE(src->backingStoreRaw); + VIR_FREE(src->dataFileRaw); return -2; } } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 1514991c20..4c9272150c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -499,7 +499,7 @@ mymain(void) VIR_STORAGE_FILE_QCOW2, EXP_PASS); TEST_CHAIN("qcow2datafile-qcow2_qcow2-datafile-auto", abs_srcdir "/virstoragetestdata/images/qcow2_datafile-auto.qcow2", - VIR_STORAGE_FILE_QCOW2, EXP_PASS); + VIR_STORAGE_FILE_QCOW2, EXP_FAIL); /* broken qcow2 with a 'data_file' which is an empty string */ TEST_CHAIN("qcow2-datafile-broken", diff --git a/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto b/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto deleted file mode 100644 index 421e0c8d5a..0000000000 --- a/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile-auto +++ /dev/null @@ -1,31 +0,0 @@ -path:ABS_SRCDIR/virstoragetestdata/images/qcow2_datafile-auto.qcow2 -backingStoreRaw: datafile.qcow2 -backingStoreRawFormat: <null>(-1) -dataFileRaw: <null> -capacity: 1024 -encryption: 0 -relPath:<null> -type:file -format:qcow2 -protocol:none -hostname:<null> - -path:ABS_SRCDIR/virstoragetestdata/images/datafile.qcow2 -backingStoreRaw: <null> -backingStoreRawFormat: none(0) -dataFileRaw: raw -capacity: 1024 -encryption: 0 -relPath:datafile.qcow2 -type:file -format:qcow2 -protocol:none -hostname:<null> - - dataFileStoreSource for 'ABS_SRCDIR/virstoragetestdata/images/datafile.qcow2': - path: ABS_SRCDIR/virstoragetestdata/images/raw - capacity: 0 - encryption: 0 - type:file - format:raw - -- 2.55.0
On a Tuesday in 2026, Peter Krempa via Devel wrote:
Patch 1 adds an counter-example test and patch 2 then fixes the bug.
Peter Krempa (2): virstoragetest: Add example of a qcow2 image with a backing image pointing to a data_file without 'backing_format' virStorageSourceGetMetadataRecurse: Fix format probing exception for images with 'data_file'
src/storage_file/storage_source.c | 3 ++- tests/virstoragetest.c | 7 ++++++- .../images/qcow2_datafile-auto.qcow2 | Bin 0 -> 196616 bytes 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 tests/virstoragetestdata/images/qcow2_datafile-auto.qcow2
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa