Skip to content

Commit

Permalink
vmdk: Fix possible segfault with non-VMDK backing
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
XanClic committed Jul 9, 2018
1 parent 6d6bcc4 commit 439e89f
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions block/vmdk.c
Expand Up @@ -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;
Expand Down

0 comments on commit 439e89f

Please sign in to comment.