From 3f910692c287e1c611c00e763ebeb95ed0e017f8 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Mon, 7 Aug 2017 08:38:19 -0400 Subject: block/vhdx: check error return of bdrv_getlength() Calls to bdrv_getlength() were not checking for error. In vhdx.c, this can lead to truncating an image file, so it is a definite bug. In vhdx-log.c, the path for improper behavior is less clear, but it is best to check in any case. Some minor code movement of the log_guid intialization, as well. Reported-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/vhdx-log.c | 23 ++++++++++++++++++----- block/vhdx.c | 9 ++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) (limited to 'block') diff --git a/block/vhdx-log.c b/block/vhdx-log.c index 01278f3..2e26fd4 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -491,6 +491,7 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s, uint32_t cnt, sectors_read; uint64_t new_file_size; void *data = NULL; + int64_t file_length; VHDXLogDescEntries *desc_entries = NULL; VHDXLogEntryHeader hdr_tmp = { 0 }; @@ -510,10 +511,15 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s, if (ret < 0) { goto exit; } + file_length = bdrv_getlength(bs->file->bs); + if (file_length < 0) { + ret = file_length; + goto exit; + } /* if the log shows a FlushedFileOffset larger than our current file * size, then that means the file has been truncated / corrupted, and * we must refused to open it / use it */ - if (hdr_tmp.flushed_file_offset > bdrv_getlength(bs->file->bs)) { + if (hdr_tmp.flushed_file_offset > file_length) { ret = -EINVAL; goto exit; } @@ -543,7 +549,7 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s, goto exit; } } - if (bdrv_getlength(bs->file->bs) < desc_entries->hdr.last_file_offset) { + if (file_length < desc_entries->hdr.last_file_offset) { new_file_size = desc_entries->hdr.last_file_offset; if (new_file_size % (1024*1024)) { /* round up to nearest 1MB boundary */ @@ -851,6 +857,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, uint32_t partial_sectors = 0; uint32_t bytes_written = 0; uint64_t file_offset; + int64_t file_length; VHDXHeader *header; VHDXLogEntryHeader new_hdr; VHDXLogDescriptor *new_desc = NULL; @@ -904,6 +911,12 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, sectors += partial_sectors; + file_length = bdrv_getlength(bs->file->bs); + if (file_length < 0) { + ret = file_length; + goto exit; + } + /* sectors is now how many sectors the data itself takes, not * including the header and descriptor metadata */ @@ -913,11 +926,11 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, .sequence_number = s->log.sequence, .descriptor_count = sectors, .reserved = 0, - .flushed_file_offset = bdrv_getlength(bs->file->bs), - .last_file_offset = bdrv_getlength(bs->file->bs), + .flushed_file_offset = file_length, + .last_file_offset = file_length, + .log_guid = header->log_guid, }; - new_hdr.log_guid = header->log_guid; desc_sectors = vhdx_compute_desc_sectors(new_hdr.descriptor_count); diff --git a/block/vhdx.c b/block/vhdx.c index a9cecd2..37224b8 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1166,7 +1166,14 @@ exit: static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s, uint64_t *new_offset) { - *new_offset = bdrv_getlength(bs->file->bs); + int64_t current_len; + + current_len = bdrv_getlength(bs->file->bs); + if (current_len < 0) { + return current_len; + } + + *new_offset = current_len; /* per the spec, the address for a block is in units of 1MB */ *new_offset = ROUND_UP(*new_offset, 1024 * 1024); -- cgit v1.1