Patchwork vmdk: refuse to open higher version than supported

login
register
mail settings
Submitter Fam Zheng
Date June 9, 2013, 1:44 a.m.
Message ID <1370742255-16400-1-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/249999/
State New
Headers show

Comments

Fam Zheng - June 9, 2013, 1:44 a.m.
Refuse to open higher version for safety.

Although we try to be compatible with published VMDK spec, VMware has
newer version from ESXi 5.1 exported OVF/OVA, which we have no knowledge
what's changed in it. And it is very likely to have more new versions in
the future, so it's not safe to open them blindly.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 4 ++++
 1 file changed, 4 insertions(+)
Stefan Hajnoczi - June 10, 2013, 9:09 a.m.
On Sun, Jun 09, 2013 at 09:44:15AM +0800, Fam Zheng wrote:
> Although we try to be compatible with published VMDK spec, VMware has
> newer version from ESXi 5.1 exported OVF/OVA, which we have no knowledge
> what's changed in it. And it is very likely to have more new versions in
> the future, so it's not safe to open them blindly.

The best I could find was this high-level overview:
http://myvirtualcloud.net/?p=3829

> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/vmdk.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 608daaf..d9c2368 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -558,6 +558,10 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>          header = footer.header;
>      }
>  
> +    if (le32_to_cpu(header.version) >= 3) {
> +        return -EINVAL;
> +    }
> +

Looks fine, the VMDK 5.0 spec says header.version may be 1 or 2.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Kevin Wolf - June 10, 2013, 9:09 a.m.
Am 09.06.2013 um 03:44 hat Fam Zheng geschrieben:
> Refuse to open higher version for safety.
> 
> Although we try to be compatible with published VMDK spec, VMware has
> newer version from ESXi 5.1 exported OVF/OVA, which we have no knowledge
> what's changed in it. And it is very likely to have more new versions in
> the future, so it's not safe to open them blindly.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Yes, it's definitely a good idea to add a check.

> @@ -558,6 +558,10 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>          header = footer.header;
>      }
>  
> +    if (le32_to_cpu(header.version) >= 3) {
> +        return -EINVAL;
> +    }
> +

Other block drivers return -ENOTSUP for this case, and also call
qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, ...) so that you get a
meaningful error message. Can you model the VMDK code after them?

Kevin

Patch

diff --git a/block/vmdk.c b/block/vmdk.c
index 608daaf..d9c2368 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -558,6 +558,10 @@  static int vmdk_open_vmdk4(BlockDriverState *bs,
         header = footer.header;
     }
 
+    if (le32_to_cpu(header.version) >= 3) {
+        return -EINVAL;
+    }
+
     l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
                         * le64_to_cpu(header.granularity);
     if (l1_entry_sectors == 0) {