* [Qemu-devel] [PATCH v2] vmdk: return ERROR when cluster sector is larger than vmdk limitation
@ 2018-03-22 6:24 yuchenlin
2018-03-22 7:41 ` Fam Zheng
0 siblings, 1 reply; 3+ messages in thread
From: yuchenlin @ 2018-03-22 6:24 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, yuchenlin
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 avoid writing the user data beyond limitation and record a useless offset
in grain table. We should return ERROR here.
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
v1->v2:
- change commit message
- check before allocating
- should be >=
- the unit is sector now
thanks
block/vmdk.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index f94c49a9c0..a1c21dbbba 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -47,6 +47,8 @@
#define VMDK4_FLAG_MARKER (1 << 17)
#define VMDK4_GD_AT_END 0xffffffffffffffffULL
+#define VMDK_EXTENT_MAX_SECTORS (4294967296)
+
#define VMDK_GTE_ZEROED 0x1
/* VMDK internal error codes */
@@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs,
return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
}
+ if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) {
+ return VMDK_ERROR;
+ }
+
cluster_sector = extent->next_cluster_sector;
extent->next_cluster_sector += extent->cluster_sectors;
--
2.16.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] vmdk: return ERROR when cluster sector is larger than vmdk limitation
2018-03-22 6:24 [Qemu-devel] [PATCH v2] vmdk: return ERROR when cluster sector is larger than vmdk limitation yuchenlin
@ 2018-03-22 7:41 ` Fam Zheng
2018-03-22 13:07 ` Eric Blake
0 siblings, 1 reply; 3+ messages in thread
From: Fam Zheng @ 2018-03-22 7:41 UTC (permalink / raw)
To: yuchenlin; +Cc: qemu-devel, kwolf, mreitz
On Thu, 03/22 14:24, 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 avoid writing the user data beyond limitation and record a useless offset
> in grain table. We should return ERROR here.
>
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
> v1->v2:
> - change commit message
> - check before allocating
> - should be >=
> - the unit is sector now
>
> thanks
>
> block/vmdk.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index f94c49a9c0..a1c21dbbba 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -47,6 +47,8 @@
> #define VMDK4_FLAG_MARKER (1 << 17)
> #define VMDK4_GD_AT_END 0xffffffffffffffffULL
>
> +#define VMDK_EXTENT_MAX_SECTORS (4294967296)
> +
> #define VMDK_GTE_ZEROED 0x1
>
> /* VMDK internal error codes */
> @@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs,
> return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
> }
>
> + if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) {
> + return VMDK_ERROR;
> + }
> +
> cluster_sector = extent->next_cluster_sector;
> extent->next_cluster_sector += extent->cluster_sectors;
>
> --
> 2.16.2
>
Cc'ing Kevin and Max who merge block/ patches.
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] vmdk: return ERROR when cluster sector is larger than vmdk limitation
2018-03-22 7:41 ` Fam Zheng
@ 2018-03-22 13:07 ` Eric Blake
0 siblings, 0 replies; 3+ messages in thread
From: Eric Blake @ 2018-03-22 13:07 UTC (permalink / raw)
To: Fam Zheng, yuchenlin; +Cc: kwolf, qemu-devel, mreitz, qemu block
On 03/22/2018 02:41 AM, Fam Zheng wrote:
> On Thu, 03/22 14:24, 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 avoid writing the user data beyond limitation and record a useless offset
>> in grain table. We should return ERROR here.
>>
>> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>> ---
>> +++ b/block/vmdk.c
>> @@ -47,6 +47,8 @@
>> #define VMDK4_FLAG_MARKER (1 << 17)
>> #define VMDK4_GD_AT_END 0xffffffffffffffffULL
>>
>> +#define VMDK_EXTENT_MAX_SECTORS (4294967296)
>> +
Hmm, with sectors instead of bytes, you don't want to use Phillipe's '2
* T_BYTE' macro after all; but it would still look better spelled as
'(1ULL << 32)' or '(4ULL * 1024 * 1024 * 1024)'. Most importantly,
since the value does NOT fit in a 32-bit unsigned integer, I think you
NEED to use the ULL suffix to make your intent clear.
>> #define VMDK_GTE_ZEROED 0x1
>>
>> /* VMDK internal error codes */
>> @@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs,
>> return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
>> }
>>
>> + if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) {
>> + return VMDK_ERROR;
>> + }
>> +
>> cluster_sector = extent->next_cluster_sector;
>> extent->next_cluster_sector += extent->cluster_sectors;
>>
>> --
>> 2.16.2
>>
>
> Cc'ing Kevin and Max who merge block/ patches.
Also, CC'ing qemu-block, as all block-related patches should go through
there.
>
> Reviewed-by: Fam Zheng <famz@redhat.com>
>
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-22 13:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 6:24 [Qemu-devel] [PATCH v2] vmdk: return ERROR when cluster sector is larger than vmdk limitation yuchenlin
2018-03-22 7:41 ` Fam Zheng
2018-03-22 13:07 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.