WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH]: xl: don't segfault parsing disk configs, support NU

To: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH]: xl: don't segfault parsing disk configs, support NULL physpath and ioemu:
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Thu, 19 Aug 2010 15:34:23 +0100
Cc: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>, Christoph Egger <Christoph.Egger@xxxxxxx>
Delivery-date: Thu, 19 Aug 2010 07:40:25 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Switch to a state machine parser since it's easier to handle all these
exotic cases without segfaulting. NULL physpaths are now allowed and a
dodgy hack is introduced to skip over the "ioemu:" prefix for a
virtpath. Also fixes a leak of buf2.

Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>

diff -r 6bd04080ab99 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Wed Aug 18 18:06:10 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Thu Aug 19 15:34:20 2010 +0100
@@ -539,6 +539,134 @@ static int parse_action_on_shutdown(cons
     return 0;
 }
 
+#define DSTATE_INITIAL   0
+#define DSTATE_TAP       1
+#define DSTATE_PHYSPATH  2
+#define DSTATE_VIRTPATH  3
+#define DSTATE_VIRTTYPE  4
+#define DSTATE_RW        5
+#define DSTATE_TERMINAL  6
+
+static int parse_disk_config(libxl_device_disk *disk, char *buf2)
+{
+    int state = DSTATE_INITIAL;
+    char *p, *end, *tok;
+
+    memset(disk, 0, sizeof(*disk));
+
+    for(tok = p = buf2, end = buf2 + strlen(buf2) + 1; p < end; p++) {
+        switch(state){
+        case DSTATE_INITIAL:
+            if ( *p == ':' ) {
+                *p = '\0';
+                if ( !strcmp(tok, "phy") ) {
+                    state = DSTATE_PHYSPATH;
+                    disk->phystype = PHYSTYPE_PHY;
+                }else if ( !strcmp(tok, "file") ) {
+                    state = DSTATE_PHYSPATH;
+                    disk->phystype = PHYSTYPE_FILE;
+                }else if ( !strcmp(tok, "tap") ) {
+                    state = DSTATE_TAP;
+                }else{
+                    fprintf(stderr, "Unknown disk type: %s\n", tok);
+                    return 0;
+                }
+                tok = p + 1;
+            }
+            break;
+        case DSTATE_TAP:
+            if ( *p == ':' ) {
+                *p = '\0';
+                if ( !strcmp(tok, "aio") ) {
+                    disk->phystype = PHYSTYPE_AIO;
+                }else if ( !strcmp(tok, "vhd") ) {
+                    disk->phystype = PHYSTYPE_VHD;
+                }else if ( !strcmp(tok, "qcow") ) {
+                    disk->phystype = PHYSTYPE_QCOW;
+                }else if ( !strcmp(tok, "qcow2") ) {
+                    disk->phystype = PHYSTYPE_QCOW2;
+                }else {
+                    fprintf(stderr, "Unknown tapdisk type: %s\n", tok);
+                    return 0;
+                }
+
+                tok = p + 1;
+                state = DSTATE_PHYSPATH;
+            }
+            break;
+        case DSTATE_PHYSPATH:
+            if ( *p == ',' ) {
+                int ioemu_len;
+
+                *p = '\0';
+                disk->physpath = (*tok) ? strdup(tok) : NULL;
+                tok = p + 1;
+
+                /* hack for ioemu disk spec */
+                ioemu_len = strlen("ioemu:");
+                state = DSTATE_VIRTPATH;
+                if ( tok + ioemu_len < end &&
+                    !strncmp(tok, "ioemu:", ioemu_len)) {
+                    tok += ioemu_len;
+                    p += ioemu_len;
+                }
+            }
+            break;
+        case DSTATE_VIRTPATH:
+            if ( *p == ',' || *p == ':' || *p == '\0' ) {
+                switch(*p) {
+                case ':':
+                    state = DSTATE_VIRTTYPE;
+                    break;
+                case ',':
+                    state = DSTATE_RW;
+                    break;
+                case '\0':
+                    state = DSTATE_TERMINAL;
+                    break;
+                }
+                if ( tok == p )
+                    goto out;
+                *p = '\0';
+                disk->virtpath = (*tok) ? strdup(tok) : NULL;
+                tok = p + 1;
+            }
+            break;
+        case DSTATE_VIRTTYPE:
+            if ( *p == ',' || *p == '\0' ) {
+                *p = '\0';
+                if ( !strcmp(tok, "cdrom") ) {
+                    disk->is_cdrom = 1;
+                    disk->unpluggable = 1;
+                }else{
+                    fprintf(stderr, "Unknown virtual disk type: %s\n", tok);
+                    return 0;
+                }
+                tok = p + 1;
+                state = (*p == ',') ? DSTATE_RW : DSTATE_TERMINAL;
+            }
+            break;
+        case DSTATE_RW:
+            if ( *p == '\0' ) {
+                disk->readwrite = (tok[0] == 'w');
+                tok = p + 1;
+                state = DSTATE_TERMINAL;
+            }
+            break;
+        case DSTATE_TERMINAL:
+            goto out;
+        }
+    }
+
+out:
+    if ( tok != p || state != DSTATE_TERMINAL ) {
+        fprintf(stderr, "parse error in disk config near '%s'\n", tok);
+        return 0;
+    }
+
+    return 1;
+}
+
 static void parse_config_data(const char *configfile_filename_report,
                               const char *configfile_data,
                               int configfile_len,
@@ -716,59 +844,13 @@ static void parse_config_data(const char
         while ((buf = xlu_cfg_get_listitem (vbds, d_config->num_disks)) != 
NULL) {
             libxl_device_disk *disk;
             char *buf2 = strdup(buf);
-            char *p, *p2;
 
             d_config->disks = (libxl_device_disk *) realloc(d_config->disks, 
sizeof (libxl_device_disk) * (d_config->num_disks + 1));
             disk = d_config->disks + d_config->num_disks;
-
-            disk->backend_domid = 0;
-            disk->domid = 0;
-            disk->unpluggable = 0;
-
-            p = strtok(buf2, ",:");
-            while (*p == ' ')
-                p++;
-            if (!strcmp(p, "phy")) {
-                disk->phystype = PHYSTYPE_PHY;
-            } else if (!strcmp(p, "file")) {
-                disk->phystype = PHYSTYPE_FILE;
-            } else if (!strcmp(p, "tap")) {
-                p = strtok(NULL, ":");
-                if (!strcmp(p, "aio")) {
-                    disk->phystype = PHYSTYPE_AIO;
-                } else if (!strcmp(p, "vhd")) {
-                    disk->phystype = PHYSTYPE_VHD;
-                } else if (!strcmp(p, "qcow")) {
-                    disk->phystype = PHYSTYPE_QCOW;
-                } else if (!strcmp(p, "qcow2")) {
-                    disk->phystype = PHYSTYPE_QCOW2;
-                }
+            if ( !parse_disk_config(disk, buf2) ) {
+                exit(1);
             }
-            p = strtok(NULL, ",");
-            while (*p == ' ')
-                p++;
-            disk->physpath= strdup(p);
-            p = strtok(NULL, ",");
-            while (*p == ' ')
-                p++;
-            p2 = strchr(p, ':');
-            if (p2 == NULL) {
-                disk->virtpath = strdup(p);
-                disk->is_cdrom = 0;
-                disk->unpluggable = 1;
-            } else {
-                *p2 = '\0';
-                disk->virtpath = strdup(p);
-                if (!strcmp(p2 + 1, "cdrom")) {
-                    disk->is_cdrom = 1;
-                    disk->unpluggable = 1;
-                } else
-                    disk->is_cdrom = 0;
-            }
-            p = strtok(NULL, ",");
-            while (*p == ' ')
-                p++;
-            disk->readwrite = (p[0] == 'w') ? 1 : 0;
+
             free(buf2);
             d_config->num_disks++;
         }



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel