Re: [Qemu-devel] [PATCH] vmdk: return ENOTSUP before offset overflow

[Fam Zheng <famz@redhat.com>]

On Thu, 03/22 10:40, yuchenlin@synology.com wrote:
> From: yuchenlin <yuchenlin@synology.com>
> 
> VMDK has a hard limitation of extent size, which is due to the size of grain
> table entry is 32 bits. It means it can only point to a grain located at
> offset = 2^32. To prevent offset overflow and record a useless offset
> in grain table. We should return un-support here.
> 
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
>  block/vmdk.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index f94c49a9c0..d8fc961940 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -47,6 +47,9 @@
>  #define VMDK4_FLAG_MARKER (1 << 17)
>  #define VMDK4_GD_AT_END 0xffffffffffffffffULL
>  
> +/* 2TB */
> +#define VMDK_EXTENT_SIZE_LIMIT (2199023255552)
> +
>  #define VMDK_GTE_ZEROED 0x1
>  
>  /* VMDK internal error codes */
> @@ -1645,6 +1648,9 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
>                  return ret;
>              }
>              if (m_data.valid) {
> +                if (cluster_offset > VMDK_EXTENT_SIZE_LIMIT) {

I think this should be >=?

Also the check should be done earlier, in get_cluster_offset even before
m_data.valid is set. We can peek at extent->next_cluster_sector to tell if the
allocation will succeed or not, and avoid writing the user data.

Fam

> +                    return -ENOTSUP;
> +                }
>                  /* update L2 tables */
>                  if (vmdk_L2update(extent, &m_data,
>                                    cluster_offset >> BDRV_SECTOR_BITS)
> -- 
> 2.16.2
>