]> xenbits.xensource.com Git - qemu-xen-4.0-testing.git/commitdiff
fix disk format security vulnerability; do not guess format for qcow vbds
authorIan Jackson <iwj@mariner.uk.xensource.com>
Tue, 8 Jul 2008 09:29:37 +0000 (10:29 +0100)
committerIan Jackson <Ian.Jackson@eu.citrix.com>
Tue, 8 Jul 2008 09:29:37 +0000 (10:29 +0100)
These are the accidentally dropped hunks of xen-unstable 17606 and 17646.

Particularly, 17606 is important:

  * make the xenstore reader in qemu-dm's startup determine which
    of qemu's block drivers to use according to the xenstore
    backend `type' field.  This `type' field typically comes from
    the front of the drive mapping string in ioemu.  The
    supported cases are:
      xm config file string      `type'  image format    qemu driver
       phy:[/dev/]<device>        phy     raw image       bdrv_raw
       file:<filename>            file    raw image       bdrv_raw
       tap:aio:<filename>         tap     raw image       bdrv_raw
       tap:qcow:<image>           tap     not raw         autoprobe
       tap:<cow-fmt>:<image>      tap     named format    bdrv_<cow-fmt>
    It is still necessary to autoprobe when the image is specified as
    `tap:qcow:<image>', because qemu distinguishes `qcow' and `qcow2'
    whereas blktap doesn't; `qcow' in xenstore typically means what
    qemu calls qcow2.  This is OK because qemu can safely distinguish
    the different cow formats provided we know it's not a raw image.

xenstore.c

index 943d8195ab5b8ea1504951bd6090b60b4ebd0703..78f39d037c0c4c26feb8351cc80b1d3664666dea 100644 (file)
@@ -109,6 +109,7 @@ void xenstore_parse_domain_config(int hvm_domid)
     int i, is_scsi, is_hdN = 0;
     unsigned int len, num, hd_index, pci_devid = 0;
     BlockDriverState *bs;
+    BlockDriver *format;
 
 #ifdef  FIXME_MEDIA_EJECT /* not yet supported in this merge */
     for(i = 0; i < MAX_DISKS + MAX_SCSI_DISKS; i++)
@@ -156,6 +157,8 @@ void xenstore_parse_domain_config(int hvm_domid)
     }
         
     for (i = 0; i < num; i++) {
+       format = NULL; /* don't know what the format is yet */
+
         /* read the backend path */
         if (pasprintf(&buf, "%s/device/vbd/%s/backend", path, e[i]) == -1)
             continue;
@@ -208,13 +211,20 @@ void xenstore_parse_domain_config(int hvm_domid)
         drv = xs_read(xsh, XBT_NULL, buf, &len);
         if (drv == NULL)
             continue;
-        /* Strip off blktap sub-type prefix aio: - QEMU can autodetect this */
+        /* Obtain blktap sub-type prefix */
         if (!strcmp(drv, "tap") && params[0]) {
             char *offset = strchr(params, ':'); 
             if (!offset)
                 continue ;
+           free(drv);
+           drv = malloc(offset - params + 1);
+           memcpy(drv, params, offset - params);
+           drv[offset - params] = '\0';
+           if (!strcmp(drv, "aio"))
+               /* qemu does aio anyway if it can */
+               format = &bdrv_raw;
             memmove(params, offset+1, strlen(offset+1)+1 );
-            fprintf(logfile, "Strip off blktap sub-type prefix to %s\n", params); 
+            fprintf(logfile, "Strip off blktap sub-type prefix to %s (drv '%s')\n", params, drv); 
         }
         /* Prefix with /dev/ if needed */
         if (!strcmp(drv, "phy") && params[0] != '/') {
@@ -222,6 +232,7 @@ void xenstore_parse_domain_config(int hvm_domid)
             sprintf(newparams, "/dev/%s", params);
             free(params);
             params = newparams;
+           format = &bdrv_raw;
         }
 
         /* 
@@ -267,8 +278,27 @@ void xenstore_parse_domain_config(int hvm_domid)
 #endif
 
         if (params[0]) {
-            if (bdrv_open(bs, params, 0 /* snapshot */) < 0)
-                fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s'\n", buf, params);
+           if (!format) {
+               if (!drv) {
+                   fprintf(stderr, "qemu: type (image format) not specified for vbd '%s' or image '%s'\n", buf, params);
+                   continue;
+               }
+               if (!strcmp(drv,"qcow")) {
+                   /* autoguess qcow vs qcow2 */
+               } else if (!strcmp(drv,"file")) {
+                   format = &bdrv_raw;
+               } else if (!strcmp(drv,"phy")) {
+                   format = &bdrv_raw;
+               } else {
+                   format = bdrv_find_format(drv);
+                   if (!format) {
+                       fprintf(stderr, "qemu: type (image format) '%s' unknown for vbd '%s' or image '%s'\n", drv, buf, params);
+                       continue;
+                   }
+               }
+           }
+            if (bdrv_open2(bs, params, 0 /* snapshot */, format) < 0)
+                fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s' (drv '%s' format '%s')\n", buf, params, drv ? drv : "?", format ? format->format_name : "0");
         }
 
        assert(nb_drives<MAX_DRIVES);