[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH] block/vhdx: add check for truncated image files

From: Peter Lieven
Subject: Re: [Qemu-block] [PATCH] block/vhdx: add check for truncated image files
Date: Mon, 2 Sep 2019 15:15:40 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

Am 02.09.19 um 15:07 schrieb Kevin Wolf:
Am 29.08.2019 um 15:36 hat Peter Lieven geschrieben:
qemu is currently not able to detect truncated vhdx image files.
Add a basic check if all allocated blocks are reachable to vhdx_co_check.

Signed-off-by: Jan-Hendrik Frintrop <address@hidden>
Signed-off-by: Peter Lieven <address@hidden>
  block/vhdx.c | 19 +++++++++++++++++++
  1 file changed, 19 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index 6a09d0a55c..4382b1375d 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -2068,10 +2068,29 @@ static int coroutine_fn vhdx_co_check(BlockDriverState 
                                        BdrvCheckMode fix)
      BDRVVHDXState *s = bs->opaque;
+    VHDXSectorInfo sinfo;
+    int64_t file_size = bdrv_get_allocated_file_size(bs);
Don't you mean bdrv_getlength()?

bdrv_get_allocated_file_size() is only the allocated size, i.e. without
holes. So a higher offset may actually be present.

Isn't bdrv_getlength the virtual disk size? I need to check if a block

points to a location after EOF of the underlying physical file.

+    int64_t sector_num;
if (s->log_replayed_on_open) {
+    for (sector_num = 0; sector_num < bs->total_sectors;
+         sector_num += s->block_size / BDRV_SECTOR_SIZE) {
+        int nb_sectors = MIN(bs->total_sectors - sector_num,
+                             s->block_size / BDRV_SECTOR_SIZE);
+        vhdx_block_translate(s, sector_num, nb_sectors, &sinfo);
+        if ((s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) ==
+            if (sinfo.file_offset +
+                sinfo.sectors_avail * BDRV_SECTOR_SIZE > file_size) {
Do we need to protect against integer overflows here? I think
sinfo.file_offset comes directly from the image file and might be

Or has it already been check somewhere?

The headers are being checked in vhdx_open.  sinfo.file_offset + 
sinfo.sectors_avail * BDRV_SECTOR_SIZE

is exactly what is being passed to bdrv_pread when reading from the image file.

+                /* block is past the end of file, image has been truncated. */
+                result->corruptions++;
I think we should print an error message like other formats do, so that
the user knows which kind of corruption 'qemu-img check' found (include
the guest and host offset of the invalid block).

What would be the appropriate way to do this? There is no errp in the

check funtion. Inclunde headers so that error_report() is available?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]