block/vhdx.c | 102 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 17 deletions(-)
qemu is currently not able to detect truncated vhdx image files.
Add a basic check if all allocated blocks are reachable at open and
report all errors during bdrv_co_check.
Signed-off-by: Peter Lieven <pl@kamp.de>
---
V3: - check for bdrv_getlength failure [Kevin]
- use uint32_t for i [Kevin]
- check for BAT entry overflow [Kevin]
- break on !errcnt in second check
V2: - add error reporting [Kevin]
- use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
- factor out BAT entry check and add error reporting for region
overlaps
- already check on vhdx_open
block/vhdx.c | 102 ++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 85 insertions(+), 17 deletions(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 6a09d0a55c..253e32d524 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -24,6 +24,7 @@
#include "qemu/option.h"
#include "qemu/crc32c.h"
#include "qemu/bswap.h"
+#include "qemu/error-report.h"
#include "vhdx.h"
#include "migration/blocker.h"
#include "qemu/uuid.h"
@@ -235,6 +236,9 @@ static int vhdx_region_check(BDRVVHDXState *s, uint64_t start, uint64_t length)
end = start + length;
QLIST_FOREACH(r, &s->regions, entries) {
if (!((start >= r->end) || (end <= r->start))) {
+ error_report("VHDX region %" PRIu64 "-%" PRIu64 " overlaps with "
+ "region %" PRIu64 "-%." PRIu64, start, end, r->start,
+ r->end);
ret = -EINVAL;
goto exit;
}
@@ -877,6 +881,77 @@ static void vhdx_calc_bat_entries(BDRVVHDXState *s)
}
+static int vhdx_check_bat_entries(BlockDriverState *bs, int *errcnt)
+{
+ BDRVVHDXState *s = bs->opaque;
+ int64_t image_file_size = bdrv_getlength(bs->file->bs);
+ uint64_t payblocks = s->chunk_ratio;
+ uint32_t i;
+ int ret = 0;
+
+ if (image_file_size < 0) {
+ error_report("Could not determinate VHDX image file size.");
+ return image_file_size;
+ }
+
+ for (i = 0; i < s->bat_entries; i++) {
+ if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) ==
+ PAYLOAD_BLOCK_FULLY_PRESENT) {
+ uint64_t offset = s->bat[i] & VHDX_BAT_FILE_OFF_MASK;
+ /*
+ * Check for BAT entry overflow.
+ */
+ if (offset > INT64_MAX - s->block_size) {
+ error_report("VHDX BAT entry %" PRIu32 " offset overflow.", i);
+ ret = -EINVAL;
+ if (!errcnt) {
+ break;
+ }
+ (*errcnt)++;
+ }
+ /*
+ * Check if fully allocated BAT entries do not reside after
+ * end of the image file.
+ */
+ if (offset + s->block_size > image_file_size) {
+ error_report("VHDX BAT entry %" PRIu32 " offset points after "
+ "end of file. Image has probably been truncated.",
+ i);
+ ret = -EINVAL;
+ if (!errcnt) {
+ break;
+ }
+ (*errcnt)++;
+ }
+
+ /*
+ * verify populated BAT field file offsets against
+ * region table and log entries
+ */
+ if (payblocks--) {
+ /* payload bat entries */
+ int ret2;
+ ret2 = vhdx_region_check(s, offset, s->block_size);
+ if (ret2 < 0) {
+ ret = -EINVAL;
+ if (!errcnt) {
+ break;
+ }
+ (*errcnt)++;
+ }
+ } else {
+ payblocks = s->chunk_ratio;
+ /*
+ * Once differencing files are supported, verify sector bitmap
+ * blocks here
+ */
+ }
+ }
+ }
+
+ return ret;
+}
+
static void vhdx_close(BlockDriverState *bs)
{
BDRVVHDXState *s = bs->opaque;
@@ -981,25 +1056,15 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- uint64_t payblocks = s->chunk_ratio;
- /* endian convert, and verify populated BAT field file offsets against
- * region table and log entries */
+ /* endian convert populated BAT field entires */
for (i = 0; i < s->bat_entries; i++) {
s->bat[i] = le64_to_cpu(s->bat[i]);
- if (payblocks--) {
- /* payload bat entries */
- if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) ==
- PAYLOAD_BLOCK_FULLY_PRESENT) {
- ret = vhdx_region_check(s, s->bat[i] & VHDX_BAT_FILE_OFF_MASK,
- s->block_size);
- if (ret < 0) {
- goto fail;
- }
- }
- } else {
- payblocks = s->chunk_ratio;
- /* Once differencing files are supported, verify sector bitmap
- * blocks here */
+ }
+
+ if (!(flags & BDRV_O_CHECK)) {
+ ret = vhdx_check_bat_entries(bs, NULL);
+ if (ret < 0) {
+ goto fail;
}
}
@@ -2072,6 +2137,9 @@ static int coroutine_fn vhdx_co_check(BlockDriverState *bs,
if (s->log_replayed_on_open) {
result->corruptions_fixed++;
}
+
+ vhdx_check_bat_entries(bs, &result->corruptions);
+
return 0;
}
--
2.17.1
Am 03.09.2019 um 15:35 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 at open and
> report all errors during bdrv_co_check.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> V3: - check for bdrv_getlength failure [Kevin]
> - use uint32_t for i [Kevin]
> - check for BAT entry overflow [Kevin]
> - break on !errcnt in second check
>
> V2: - add error reporting [Kevin]
> - use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
> - factor out BAT entry check and add error reporting for region
> overlaps
> - already check on vhdx_open
Something still seems to be wrong with this patch:
213 fail [15:50:13] [15:50:14] (last: 2s) output mismatch (see 213.out.bad)
--- /home/kwolf/source/qemu/tests/qemu-iotests/213.out 2019-06-28 14:19:50.065797707 +0200
+++ /home/kwolf/source/qemu/tests/qemu-iotests/213.out.bad 2019-09-04 15:50:14.582053976 +0200
@@ -46,10 +46,8 @@
{"execute": "job-dismiss", "arguments": {"id": "job0"}}
{"return": {}}
-image: TEST_IMG
-file format: IMGFMT
-virtual size: 32 MiB (33554432 bytes)
-cluster_size: 268435456
+qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
+qemu-img: Could not open 'TEST_IMG': Could not open 'TEST_IMG': Invalid argument
=== Invalid BlockdevRef ===
I can reproduce this manually with the following qemu-img invocations.
It seems all three options must be given to reproduce the error:
$ ./qemu-img create -f vhdx -o block_size=268435456,subformat=fixed,block_state_zero=off /tmp/test.vhdx 32M
Formatting '/tmp/test.vhdx', fmt=vhdx size=33554432 log_size=1048576 block_size=268435456 subformat=fixed block_state_zero=off
$ ./qemu-img info /tmp/test.vhdx
qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
qemu-img: Could not open '/tmp/test.vhdx': Could not open '/tmp/test.vhdx': Invalid argument
If I add the offsets to the error message (would probably nice to have),
I get:
qemu-img: VHDX BAT entry 0 offset 8388608 points after end of file (41943040). Image has probably been truncated.
So it seems that the file is large enough to hold 32M + metadata, but we
don't increase the file size to hold a full block (256M). Is this a
problem in the way we create images or are partial blocks at the end
expected?
Kevin
Am 04.09.19 um 16:09 schrieb Kevin Wolf:
> Am 03.09.2019 um 15:35 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 at open and
>> report all errors during bdrv_co_check.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> V3: - check for bdrv_getlength failure [Kevin]
>> - use uint32_t for i [Kevin]
>> - check for BAT entry overflow [Kevin]
>> - break on !errcnt in second check
>>
>> V2: - add error reporting [Kevin]
>> - use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
>> - factor out BAT entry check and add error reporting for region
>> overlaps
>> - already check on vhdx_open
> Something still seems to be wrong with this patch:
>
> 213 fail [15:50:13] [15:50:14] (last: 2s) output mismatch (see 213.out.bad)
> --- /home/kwolf/source/qemu/tests/qemu-iotests/213.out 2019-06-28 14:19:50.065797707 +0200
> +++ /home/kwolf/source/qemu/tests/qemu-iotests/213.out.bad 2019-09-04 15:50:14.582053976 +0200
> @@ -46,10 +46,8 @@
> {"execute": "job-dismiss", "arguments": {"id": "job0"}}
> {"return": {}}
>
> -image: TEST_IMG
> -file format: IMGFMT
> -virtual size: 32 MiB (33554432 bytes)
> -cluster_size: 268435456
> +qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
> +qemu-img: Could not open 'TEST_IMG': Could not open 'TEST_IMG': Invalid argument
>
> === Invalid BlockdevRef ===
>
> I can reproduce this manually with the following qemu-img invocations.
> It seems all three options must be given to reproduce the error:
>
> $ ./qemu-img create -f vhdx -o block_size=268435456,subformat=fixed,block_state_zero=off /tmp/test.vhdx 32M
> Formatting '/tmp/test.vhdx', fmt=vhdx size=33554432 log_size=1048576 block_size=268435456 subformat=fixed block_state_zero=off
> $ ./qemu-img info /tmp/test.vhdx
> qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
> qemu-img: Could not open '/tmp/test.vhdx': Could not open '/tmp/test.vhdx': Invalid argument
>
> If I add the offsets to the error message (would probably nice to have),
> I get:
>
> qemu-img: VHDX BAT entry 0 offset 8388608 points after end of file (41943040). Image has probably been truncated.
I can add this info and maybe we should also distinguish if the block is completely or only partially after
the end of file.
>
> So it seems that the file is large enough to hold 32M + metadata, but we
> don't increase the file size to hold a full block (256M). Is this a
> problem in the way we create images or are partial blocks at the end
> expected?
I have no idea if partial blocks are allowed. In all image created by Hyper-V that I used
to test the checker has reported no error.
Best,
Peter
Am 04.09.19 um 16:09 schrieb Kevin Wolf:
> Am 03.09.2019 um 15:35 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 at open and
>> report all errors during bdrv_co_check.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> V3: - check for bdrv_getlength failure [Kevin]
>> - use uint32_t for i [Kevin]
>> - check for BAT entry overflow [Kevin]
>> - break on !errcnt in second check
>>
>> V2: - add error reporting [Kevin]
>> - use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
>> - factor out BAT entry check and add error reporting for region
>> overlaps
>> - already check on vhdx_open
> Something still seems to be wrong with this patch:
>
> 213 fail [15:50:13] [15:50:14] (last: 2s) output mismatch (see 213.out.bad)
> --- /home/kwolf/source/qemu/tests/qemu-iotests/213.out 2019-06-28 14:19:50.065797707 +0200
> +++ /home/kwolf/source/qemu/tests/qemu-iotests/213.out.bad 2019-09-04 15:50:14.582053976 +0200
> @@ -46,10 +46,8 @@
> {"execute": "job-dismiss", "arguments": {"id": "job0"}}
> {"return": {}}
>
> -image: TEST_IMG
> -file format: IMGFMT
> -virtual size: 32 MiB (33554432 bytes)
> -cluster_size: 268435456
> +qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
> +qemu-img: Could not open 'TEST_IMG': Could not open 'TEST_IMG': Invalid argument
>
> === Invalid BlockdevRef ===
>
> I can reproduce this manually with the following qemu-img invocations.
> It seems all three options must be given to reproduce the error:
>
> $ ./qemu-img create -f vhdx -o block_size=268435456,subformat=fixed,block_state_zero=off /tmp/test.vhdx 32M
> Formatting '/tmp/test.vhdx', fmt=vhdx size=33554432 log_size=1048576 block_size=268435456 subformat=fixed block_state_zero=off
> $ ./qemu-img info /tmp/test.vhdx
> qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
> qemu-img: Could not open '/tmp/test.vhdx': Could not open '/tmp/test.vhdx': Invalid argument
>
> If I add the offsets to the error message (would probably nice to have),
> I get:
>
> qemu-img: VHDX BAT entry 0 offset 8388608 points after end of file (41943040). Image has probably been truncated.
>
> So it seems that the file is large enough to hold 32M + metadata, but we
> don't increase the file size to hold a full block (256M). Is this a
> problem in the way we create images or are partial blocks at the end
> expected?
>
> Kevin
A short look into the VHDX spec [1] seems to suggest that a VHDX File can only grow in Block increments.
See page 8 in the definition of blocks: "Allocation of new space for a virtual hard disk that supports dynamic growth
of the virtual hard disk file is done in fixes size units defined as blocks."
Peter
[1] https://www.microsoft.com/en-us/download/confirmation.aspx?id=34750
--
Mit freundlichen Grüßen
Peter Lieven
...........................................................
KAMP Netzwerkdienste GmbH
Vestische Str. 89-91 | 46117 Oberhausen
Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
pl@kamp.de | http://www.kamp.de
Geschäftsführer: Heiner Lante | Michael Lante
Amtsgericht Duisburg | HRB Nr. 12154
USt-Id-Nr.: DE 120607556
...........................................................
Am 05.09.2019 um 12:02 hat Peter Lieven geschrieben:
> Am 04.09.19 um 16:09 schrieb Kevin Wolf:
> > Am 03.09.2019 um 15:35 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 at open and
> > > report all errors during bdrv_co_check.
> > >
> > > Signed-off-by: Peter Lieven <pl@kamp.de>
> > > ---
> > > V3: - check for bdrv_getlength failure [Kevin]
> > > - use uint32_t for i [Kevin]
> > > - check for BAT entry overflow [Kevin]
> > > - break on !errcnt in second check
> > >
> > > V2: - add error reporting [Kevin]
> > > - use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
> > > - factor out BAT entry check and add error reporting for region
> > > overlaps
> > > - already check on vhdx_open
> > Something still seems to be wrong with this patch:
> >
> > 213 fail [15:50:13] [15:50:14] (last: 2s) output mismatch (see 213.out.bad)
> > --- /home/kwolf/source/qemu/tests/qemu-iotests/213.out 2019-06-28 14:19:50.065797707 +0200
> > +++ /home/kwolf/source/qemu/tests/qemu-iotests/213.out.bad 2019-09-04 15:50:14.582053976 +0200
> > @@ -46,10 +46,8 @@
> > {"execute": "job-dismiss", "arguments": {"id": "job0"}}
> > {"return": {}}
> >
> > -image: TEST_IMG
> > -file format: IMGFMT
> > -virtual size: 32 MiB (33554432 bytes)
> > -cluster_size: 268435456
> > +qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
> > +qemu-img: Could not open 'TEST_IMG': Could not open 'TEST_IMG': Invalid argument
> >
> > === Invalid BlockdevRef ===
> >
> > I can reproduce this manually with the following qemu-img invocations.
> > It seems all three options must be given to reproduce the error:
> >
> > $ ./qemu-img create -f vhdx -o block_size=268435456,subformat=fixed,block_state_zero=off /tmp/test.vhdx 32M
> > Formatting '/tmp/test.vhdx', fmt=vhdx size=33554432 log_size=1048576 block_size=268435456 subformat=fixed block_state_zero=off
> > $ ./qemu-img info /tmp/test.vhdx
> > qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
> > qemu-img: Could not open '/tmp/test.vhdx': Could not open '/tmp/test.vhdx': Invalid argument
> >
> > If I add the offsets to the error message (would probably nice to have),
> > I get:
> >
> > qemu-img: VHDX BAT entry 0 offset 8388608 points after end of file (41943040). Image has probably been truncated.
> >
> > So it seems that the file is large enough to hold 32M + metadata, but we
> > don't increase the file size to hold a full block (256M). Is this a
> > problem in the way we create images or are partial blocks at the end
> > expected?
> >
> > Kevin
>
>
> A short look into the VHDX spec [1] seems to suggest that a VHDX File
> can only grow in Block increments.
>
> See page 8 in the definition of blocks: "Allocation of new space for a
> virtual hard disk that supports dynamic growth of the virtual hard
> disk file is done in fixes size units defined as blocks."
Then I guess we need to fix the creation of VHDX images before we can
apply this patch because otherwise qemu-iotests fails.
Hm... And probably ignore the error for a partial final block anyway to
maintain compatibility with images created by older QEMU versions.
Kevin
Am 10.09.19 um 13:15 schrieb Kevin Wolf:
> Am 05.09.2019 um 12:02 hat Peter Lieven geschrieben:
>> Am 04.09.19 um 16:09 schrieb Kevin Wolf:
>>> Am 03.09.2019 um 15:35 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 at open and
>>>> report all errors during bdrv_co_check.
>>>>
>>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>>> ---
>>>> V3: - check for bdrv_getlength failure [Kevin]
>>>> - use uint32_t for i [Kevin]
>>>> - check for BAT entry overflow [Kevin]
>>>> - break on !errcnt in second check
>>>>
>>>> V2: - add error reporting [Kevin]
>>>> - use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
>>>> - factor out BAT entry check and add error reporting for region
>>>> overlaps
>>>> - already check on vhdx_open
>>> Something still seems to be wrong with this patch:
>>>
>>> 213 fail [15:50:13] [15:50:14] (last: 2s) output mismatch (see 213.out.bad)
>>> --- /home/kwolf/source/qemu/tests/qemu-iotests/213.out 2019-06-28 14:19:50.065797707 +0200
>>> +++ /home/kwolf/source/qemu/tests/qemu-iotests/213.out.bad 2019-09-04 15:50:14.582053976 +0200
>>> @@ -46,10 +46,8 @@
>>> {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>>> {"return": {}}
>>>
>>> -image: TEST_IMG
>>> -file format: IMGFMT
>>> -virtual size: 32 MiB (33554432 bytes)
>>> -cluster_size: 268435456
>>> +qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
>>> +qemu-img: Could not open 'TEST_IMG': Could not open 'TEST_IMG': Invalid argument
>>>
>>> === Invalid BlockdevRef ===
>>>
>>> I can reproduce this manually with the following qemu-img invocations.
>>> It seems all three options must be given to reproduce the error:
>>>
>>> $ ./qemu-img create -f vhdx -o block_size=268435456,subformat=fixed,block_state_zero=off /tmp/test.vhdx 32M
>>> Formatting '/tmp/test.vhdx', fmt=vhdx size=33554432 log_size=1048576 block_size=268435456 subformat=fixed block_state_zero=off
>>> $ ./qemu-img info /tmp/test.vhdx
>>> qemu-img: VHDX BAT entry 0 offset points after end of file. Image has probably been truncated.
>>> qemu-img: Could not open '/tmp/test.vhdx': Could not open '/tmp/test.vhdx': Invalid argument
>>>
>>> If I add the offsets to the error message (would probably nice to have),
>>> I get:
>>>
>>> qemu-img: VHDX BAT entry 0 offset 8388608 points after end of file (41943040). Image has probably been truncated.
>>>
>>> So it seems that the file is large enough to hold 32M + metadata, but we
>>> don't increase the file size to hold a full block (256M). Is this a
>>> problem in the way we create images or are partial blocks at the end
>>> expected?
>>>
>>> Kevin
>>
>> A short look into the VHDX spec [1] seems to suggest that a VHDX File
>> can only grow in Block increments.
>>
>> See page 8 in the definition of blocks: "Allocation of new space for a
>> virtual hard disk that supports dynamic growth of the virtual hard
>> disk file is done in fixes size units defined as blocks."
> Then I guess we need to fix the creation of VHDX images before we can
> apply this patch because otherwise qemu-iotests fails.
>
> Hm... And probably ignore the error for a partial final block anyway to
> maintain compatibility with images created by older QEMU versions.
I would change the check to not fail for partial blocks at the end
of the image if the available bytes match the filesize.
I will send an update.
Peter
© 2016 - 2025 Red Hat, Inc.