Patchwork [v2,03/12] VMDK: probe for monolithicFlat images

login
register
mail settings
Submitter Feiran Zheng
Date June 24, 2011, 8:18 a.m.
Message ID <1308903507-10799-4-git-send-email-famcool@gmail.com>
Download mbox | patch
Permalink /patch/101742/
State New
Headers show

Comments

Feiran Zheng - June 24, 2011, 8:18 a.m.
From: Fam Zheng <famcool@gmail.com>

Probe as the same behavior as VMware does.
Recognize image as monolithicFlat descriptor file when the file is text
and the first effective line (not '#' leaded comment or space line) is
either 'version=1' or 'version=2'. No space or upper case charactors
accepted.

Signed-off-by: Fam Zheng <famcool@gmail.com>
---
 block/vmdk.c |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)
Stefan Hajnoczi - June 24, 2011, 9:56 a.m.
On Fri, Jun 24, 2011 at 9:18 AM,  <famcool@gmail.com> wrote:
> +        const char *p = (const char *)buf;
> +        const char *end = p + buf_size;
> +        int version = 0;
> +        while (*p && p < end) {

These short-circuit comparisions need to be the other way around.  If
p >= end then p is out-of-range and we cannot dereference it.
Therefore:
while (p < end && *p) {

Although what is the point of checking for the NUL byte?  This buffer
isn't a C string and NULs don't matter.

Please fix the other loops too.

> +            if (*p == '#') {
> +                /* skip comment line */
> +                while (*p != '\n' && p < end) {
> +                    p++;
> +                }
> +                p++;
> +                continue;
> +            }
> +            if (*p == ' ') {
> +                while (*p == ' ' && p < end) {
> +                    p++;
> +                }
> +                /* only accept blank lines before 'version=' line */
> +                if (*p != '\n') {
> +                    return 0;
> +                }

What about Windows line endings (\r\n)?

> +                p++;
> +                continue;
> +            }
> +            sscanf(p, "version=%d", &version);

This function cannot be used on p because the buffer is not
NUL-terminated.  sscanf(3) may run off the end of the buffer.

How about this:

/* Match supported versions, Windows and UNIX line endings */
if (strncmp("version=1\r\n", p, end - p) == 0 ||
    strncmp("version=1\n", p, end - p) == 0 ||
    strncmp("version=2\r\n", p, end - p) == 0) ||
    strncmp("version=2\n", p, end - p) == 0) {
    return 100;
}

Stefan

Patch

diff --git a/block/vmdk.c b/block/vmdk.c
index 0540ec5..71d81bc 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -102,10 +102,41 @@  static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
         return 0;
     magic = be32_to_cpu(*(uint32_t *)buf);
     if (magic == VMDK3_MAGIC ||
-        magic == VMDK4_MAGIC)
+        magic == VMDK4_MAGIC) {
         return 100;
-    else
-        return 0;
+    } else {
+        const char *p = (const char *)buf;
+        const char *end = p + buf_size;
+        int version = 0;
+        while (*p && p < end) {
+            if (*p == '#') {
+                /* skip comment line */
+                while (*p != '\n' && p < end) {
+                    p++;
+                }
+                p++;
+                continue;
+            }
+            if (*p == ' ') {
+                while (*p == ' ' && p < end) {
+                    p++;
+                }
+                /* only accept blank lines before 'version=' line */
+                if (*p != '\n') {
+                    return 0;
+                }
+                p++;
+                continue;
+            }
+            sscanf(p, "version=%d", &version);
+            if (version == 1 || version == 2) {
+                return 100;
+            } else {
+                return 0;
+            }
+        }
+    }
+    return 0;
 }
 
 #define CHECK_CID 1