[PATCH] block/vhdx: Prevent out-of-bounds BAT accesses with malicious vhdx files

[PATCH] block/vhdx: Prevent out-of-bounds BAT accesses with malicious vhdx files

[Thomas Huth]

virtual_disk_size is read directly from a VHDX image's metadata region
and never checked for sane sizes.  The 64 TiB limit (VHDX_MAX_IMAGE_SIZE)
is only enforced in vhdx_co_create(), so it limits images QEMU creates
but never images QEMU opens.

Without the bound, vhdx_calc_bat_entries() computes the block count
into a uint32_t, so a huge image size gets truncated and the BAT is
undersized relative to total_sectors.
vhdx_block_translate() then converts sector numbers into BAT indices
with no bounds check, allowing out-of-bounds access on the BAT array
from both the read and write paths.

To fix the problem, add a VHDX_MAX_IMAGE_SIZE check to vhdx_parse_metadata()
so images opened for reading are subject to the same limit as images
created by QEMU.
With this limit, the calculation in vhdx_calc_bat_entries() cannot get
truncated anymore (block_size is at least 1 MiB):

 VHDX_MAX_IMAGE_SIZE / block_size = 64 TiB / 1 MiB = 64 MiB

... and 64 MiB fit nicely into 32 bits.

To be really on the safe side, let's also add an assertion in
vhdx_block_translate() to catch accesses that exceeds the allocated
entry count.

Buglink: https://gitlab.com/qemu-project/qemu/-/work_items/3762
Reported-by: Daniel Wade <danjwade95@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4470
Reported-by: Mayank Jangid (OpenSec)
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4487
Reported-by: Ken Hsu, Royce Lu, and Bo Qu of Palo Alto Networks
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4638
Reported-by: Thai Duong (Calif.io)
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4733
Reported-by: Xiang Chen
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 block/vhdx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index c16e4a00c8d..8013a90a1a4 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -815,6 +815,11 @@ vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s)
     }
 
     s->virtual_disk_size = le64_to_cpu(s->virtual_disk_size);
+    if (s->virtual_disk_size > VHDX_MAX_IMAGE_SIZE) {
+        ret = -EFBIG;
+        goto exit;
+    }
+
     s->logical_sector_size = le32_to_cpu(s->logical_sector_size);
     s->physical_sector_size = le32_to_cpu(s->physical_sector_size);
 
@@ -1149,6 +1154,7 @@ static void vhdx_block_translate(BDRVVHDXState *s, int64_t sector_num,
 
     sinfo->bytes_avail = sinfo->sectors_avail << s->logical_sector_size_bits;
 
+    assert(sinfo->bat_idx < s->bat_entries);
     sinfo->file_offset = s->bat[sinfo->bat_idx] & VHDX_BAT_FILE_OFF_MASK;
 
     sinfo->block_offset = block_offset << s->logical_sector_size_bits;
-- 
2.56.0

Acknowledgements for AI-assisted trivial bug reports (was: [PATCH] block/vhdx: Prevent out-of-bounds BAT accesses with) malicious vhdx files

[Kevin Wolf]

Am 23.10.2026 um 15:00 hat Thomas Huth geschrieben:
> virtual_disk_size is read directly from a VHDX image's metadata region
> and never checked for sane sizes.  The 64 TiB limit (VHDX_MAX_IMAGE_SIZE)
> is only enforced in vhdx_co_create(), so it limits images QEMU creates
> but never images QEMU opens.
> 
> Without the bound, vhdx_calc_bat_entries() computes the block count
> into a uint32_t, so a huge image size gets truncated and the BAT is
> undersized relative to total_sectors.
> vhdx_block_translate() then converts sector numbers into BAT indices
> with no bounds check, allowing out-of-bounds access on the BAT array
> from both the read and write paths.
> 
> To fix the problem, add a VHDX_MAX_IMAGE_SIZE check to vhdx_parse_metadata()
> so images opened for reading are subject to the same limit as images
> created by QEMU.
> With this limit, the calculation in vhdx_calc_bat_entries() cannot get
> truncated anymore (block_size is at least 1 MiB):
> 
>  VHDX_MAX_IMAGE_SIZE / block_size = 64 TiB / 1 MiB = 64 MiB
> 
> ... and 64 MiB fit nicely into 32 bits.
> 
> To be really on the safe side, let's also add an assertion in
> vhdx_block_translate() to catch accesses that exceeds the allocated
> entry count.
> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/work_items/3762
> Reported-by: Daniel Wade <danjwade95@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4470
> Reported-by: Mayank Jangid (OpenSec)
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4487
> Reported-by: Ken Hsu, Royce Lu, and Bo Qu of Palo Alto Networks
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4638
> Reported-by: Thai Duong (Calif.io)
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4733
> Reported-by: Xiang Chen

So if I understand correctly, this issue is trivial enough that it was
found by five people('s AI agents) independently, can be fixed in five
lines of code, and not a single one of the reporters could be bothered
to send an actual patch or at least attach one to their report?

All these reports seems to include two kilometers of AI output and
somewhere in it they typically hide a section on suggested fixes, but
always either in English or as a code snippet that you have to manually
insert in the right place instead of a patch that can actually be
applied.

I wonder if we're setting the wrong incentives if we reward such
laziness with Reported-by lines for each reporter. I'm inclined to drop
all of them and only leave the Resolves lines in place.

Maybe how to deal with such cases could actually be a topic for QEMU
Summit.

Kevin

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  block/vhdx.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index c16e4a00c8d..8013a90a1a4 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -815,6 +815,11 @@ vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s)
>      }
>  
>      s->virtual_disk_size = le64_to_cpu(s->virtual_disk_size);
> +    if (s->virtual_disk_size > VHDX_MAX_IMAGE_SIZE) {
> +        ret = -EFBIG;
> +        goto exit;
> +    }
> +
>      s->logical_sector_size = le32_to_cpu(s->logical_sector_size);
>      s->physical_sector_size = le32_to_cpu(s->physical_sector_size);
>  
> @@ -1149,6 +1154,7 @@ static void vhdx_block_translate(BDRVVHDXState *s, int64_t sector_num,
>  
>      sinfo->bytes_avail = sinfo->sectors_avail << s->logical_sector_size_bits;
>  
> +    assert(sinfo->bat_idx < s->bat_entries);
>      sinfo->file_offset = s->bat[sinfo->bat_idx] & VHDX_BAT_FILE_OFF_MASK;
>  
>      sinfo->block_offset = block_offset << s->logical_sector_size_bits;
> -- 
> 2.56.0
> 
> 

Re: Acknowledgements for AI-assisted trivial bug reports (was: [PATCH] block/vhdx: Prevent out-of-bounds BAT accesses with) malicious vhdx files

[Peter Maydell]

On Fri, 6 Nov 2026 at 14:50, Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 23.10.2026 um 15:00 hat Thomas Huth geschrieben:
> > Buglink: https://gitlab.com/qemu-project/qemu/-/work_items/3762
> > Reported-by: Daniel Wade <danjwade95@gmail.com>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4470
> > Reported-by: Mayank Jangid (OpenSec)
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4487
> > Reported-by: Ken Hsu, Royce Lu, and Bo Qu of Palo Alto Networks
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4638
> > Reported-by: Thai Duong (Calif.io)
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4733
> > Reported-by: Xiang Chen
>
> So if I understand correctly, this issue is trivial enough that it was
> found by five people('s AI agents) independently, can be fixed in five
> lines of code, and not a single one of the reporters could be bothered
> to send an actual patch or at least attach one to their report?

To be fair here, I am pretty sure that if any of these reporters were
going to produce a patch it would be an AI generated one, and our
current policy says we decline those.

Plus in my experience the hard part is not the code change but
doing the work to figure out if that is actually the right way
to fix the problem, what the consequences are, what any relevant
specs say, and so on. All of which we'd have to do in code review
if the patch came from somebody who doesn't already have enough
community trust about their ability to have done that.

> All these reports seems to include two kilometers of AI output and
> somewhere in it they typically hide a section on suggested fixes, but
> always either in English or as a code snippet that you have to manually
> insert in the right place instead of a patch that can actually be
> applied.
>
> I wonder if we're setting the wrong incentives if we reward such
> laziness with Reported-by lines for each reporter. I'm inclined to drop
> all of them and only leave the Resolves lines in place.

I have tended to collapse them all down where there are duplicates
and only give a reported-by to the one that reported first.
I don't think we need Resolves: tags for 5 identical bug
reports either -- we should mark all but one duplicate (which you
can do in gitlab with "/duplicate #1234" which will auto close
and link to that other bug) and only have the Resolves tag for
the one remaining open bug.

-- PMM

Re: Acknowledgements for AI-assisted trivial bug reports

[Thomas Huth]

On 06/11/2026 15.58, Peter Maydell wrote:
> On Fri, 6 Nov 2026 at 14:50, Kevin Wolf <kwolf@redhat.com> wrote:
>>
>> Am 23.10.2026 um 15:00 hat Thomas Huth geschrieben:
>>> Buglink: https://gitlab.com/qemu-project/qemu/-/work_items/3762
>>> Reported-by: Daniel Wade <danjwade95@gmail.com>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4470
>>> Reported-by: Mayank Jangid (OpenSec)
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4487
>>> Reported-by: Ken Hsu, Royce Lu, and Bo Qu of Palo Alto Networks
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4638
>>> Reported-by: Thai Duong (Calif.io)
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4733
>>> Reported-by: Xiang Chen
>>
>> So if I understand correctly, this issue is trivial enough that it was
>> found by five people('s AI agents) independently, can be fixed in five
>> lines of code, and not a single one of the reporters could be bothered
>> to send an actual patch or at least attach one to their report?
> 
> To be fair here, I am pretty sure that if any of these reporters were
> going to produce a patch it would be an AI generated one, and our
> current policy says we decline those.
> 
> Plus in my experience the hard part is not the code change but
> doing the work to figure out if that is actually the right way
> to fix the problem, what the consequences are, what any relevant
> specs say, and so on. All of which we'd have to do in code review
> if the patch came from somebody who doesn't already have enough
> community trust about their ability to have done that.
> 
>> All these reports seems to include two kilometers of AI output and
>> somewhere in it they typically hide a section on suggested fixes, but
>> always either in English or as a code snippet that you have to manually
>> insert in the right place instead of a patch that can actually be
>> applied.
>>
>> I wonder if we're setting the wrong incentives if we reward such
>> laziness with Reported-by lines for each reporter. I'm inclined to drop
>> all of them and only leave the Resolves lines in place.
> 
> I have tended to collapse them all down where there are duplicates
> and only give a reported-by to the one that reported first.
> I don't think we need Resolves: tags for 5 identical bug
> reports either -- we should mark all but one duplicate (which you
> can do in gitlab with "/duplicate #1234" which will auto close
> and link to that other bug) and only have the Resolves tag for
> the one remaining open bug.
Fine for me, let's give the credits to the first person who found it and 
mark all other tickets as duplicates.

  Thomas