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