Re: [PATCH] block/vhdx: Add alignment checks for region tables

[Kevin Wolf <kwolf@redhat.com>]

Am 12.11.2026 um 10:22 hat Thomas Huth geschrieben:
> According to the VHDX specification:
> 
>  "The FileOffset and Length fields specify the 64-bit byte offset and 32-bit
>   byte length of the object within the file. The values must be a multiple of
>   1 MB, and FileOffset must be at least 1 MB."

This requires that the offset (not length) is at least 1 MB...

> Thus let's add a sanity check to avoid that bad things could happen later.
> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/work_items/3762
> Reported-by: Daniel Wade <danjwade95@gmail.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Use QEMU_IS_ALIGNED() as suggested by Eric
> 
>  block/vhdx.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 8013a90a1a4..21270988b5a 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -566,6 +566,16 @@ vhdx_open_region_tables(BlockDriverState *bs, BDRVVHDXState *s)
>  
>          vhdx_region_entry_le_import(&rt_entry);
>  
> +        /*
> +         * According to the VHDX spec, offset and length must be multiple
> +         * of 1 MiB and length must be at least 1 MiB.
> +         */
> +        if (!QEMU_IS_ALIGNED(rt_entry.file_offset, MiB) ||
> +            !QEMU_IS_ALIGNED(rt_entry.length, MiB) || !rt_entry.length) {

...but the comment and code do the opposite and check only that the
length is at least 1 MB, leaving FileOffset (which was the actual
requirement) unchecked.

>
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +
>          /* check for region overlap between these entries, and any
>           * other memory regions in the file */
>          ret = vhdx_region_check(s, rt_entry.file_offset, rt_entry.length);

I'm also wondering if the check shouldn't be in vhdx_region_check()
instead, which we're already calling here.

Kevin