vmdk: refuse to open higher version than supported

Submitted by Fam Zheng on June 9, 2013, 1:44 a.m.

Details

Message ID 1370742255-16400-1-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

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(+)

Comments

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 hide | download patch | download mbox

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) {