aboutsummaryrefslogtreecommitdiff
path: root/block
diff options
context:
space:
mode:
authorMax Reitz <mreitz@redhat.com>2018-07-02 23:07:20 +0200
committerMax Reitz <mreitz@redhat.com>2018-07-09 19:43:24 +0200
commit439e89fc09ab6070f9613c4c513a4d4f133b23b7 (patch)
treec8f3a2d588d048b30434b99d83805991cbf2a6a0 /block
parent6d6bcc46b552c67aa14f25e69d518684b3e59361 (diff)
downloadqemu-439e89fc09ab6070f9613c4c513a4d4f133b23b7.zip
qemu-439e89fc09ab6070f9613c4c513a4d4f133b23b7.tar.gz
qemu-439e89fc09ab6070f9613c4c513a4d4f133b23b7.tar.bz2
vmdk: Fix possible segfault with non-VMDK backing
VMDK performs a probing check in vmdk_co_create_opts() to prevent the user from assigning non-VMDK files as a backing file, because it only supports VMDK backing files. However, with the @backing runtime option, it is possible to assign arbitrary nodes as backing nodes, regardless of what the image header says. Therefore, VMDK may not just access backing nodes assuming they are VMDK nodes -- which it does, because it needs to compare the backing file's CID with the overlay's parentCID value, and naturally the backing file only has a CID when it's a VMDK file. Instead, it should report the CID of non-VMDK backing files not to match the overlay because clearly a non-present CID does not match. Without this change, vmdk_read_cid() reads from the backing file's bs->file, which may be NULL (in which case we get a segfault). Also, it interprets bs->opaque as a BDRVVmdkState and then reads from the .desc_offset field, which usually will just return some arbitrary value which then results in either garbage to be read, or bdrv_pread() to return an error, both of which result in a non-matching CID to be reported. (In a very unlikely case, we could read something that looks like a VMDK descriptor, and then get a CID which might actually match. But that is highly unlikely, and the only result would be that VMDK accepts the backing file which is not too bad (albeit unintentional).) ((And in theory, the seek to .desc_offset might leak data from another block driver's opaque object. But then again, the user should realize very quickly that a non-VMDK backing file does not work (because the read will very likely fail, due to the reasons given above), so this should not be exploitable.)) Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20180702210721.4847-2-mreitz@redhat.com Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Diffstat (limited to 'block')
-rw-r--r--block/vmdk.c6
1 files changed, 6 insertions, 0 deletions
diff --git a/block/vmdk.c b/block/vmdk.c
index 84f8bbe..a9d0084 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -333,6 +333,12 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
if (!s->cid_checked && bs->backing) {
BlockDriverState *p_bs = bs->backing->bs;
+ if (strcmp(p_bs->drv->format_name, "vmdk")) {
+ /* Backing file is not in vmdk format, so it does not have
+ * a CID, which makes the overlay's parent CID invalid */
+ return 0;
+ }
+
if (vmdk_read_cid(p_bs, 0, &cur_pcid) != 0) {
/* read failure: report as not valid */
return 0;