ia64/xen-unstable

changeset 17624:e3be00bd6aa9

ioemu: fix disk format security vulnerability

* 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.

* Make the format autoprobing machinery never return `raw'. This has
two purposes: firstly, it arranges that the `tap:qcow:...' case
above can be handled without accidentally falling back to raw
format. Secondly it prevents accidents in case the code changes in
future: autoprobing will now always fail on supposed cow files which
actually contain junk, rather than giving the guest access to the
underlying file.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
author Keir Fraser <keir.fraser@citrix.com>
date Mon May 12 10:09:12 2008 +0100 (2008-05-12)
parents 4afc6023e8ec
children 3b20e543b52d
files tools/ioemu/block.c tools/ioemu/xenstore.c
line diff
     1.1 --- a/tools/ioemu/block.c	Mon May 12 10:07:26 2008 +0100
     1.2 +++ b/tools/ioemu/block.c	Mon May 12 10:09:12 2008 +0100
     1.3 @@ -254,7 +254,7 @@ static BlockDriver *find_protocol(const 
     1.4  #endif
     1.5      p = strchr(filename, ':');
     1.6      if (!p)
     1.7 -        return &bdrv_raw;
     1.8 +        return NULL; /* do not ever guess raw, it is a security problem! */
     1.9      len = p - filename;
    1.10      if (len > sizeof(protocol) - 1)
    1.11          len = sizeof(protocol) - 1;
     2.1 --- a/tools/ioemu/xenstore.c	Mon May 12 10:07:26 2008 +0100
     2.2 +++ b/tools/ioemu/xenstore.c	Mon May 12 10:09:12 2008 +0100
     2.3 @@ -90,6 +90,7 @@ void xenstore_parse_domain_config(int hv
     2.4      int i, is_scsi, is_hdN = 0;
     2.5      unsigned int len, num, hd_index, pci_devid = 0;
     2.6      BlockDriverState *bs;
     2.7 +    BlockDriver *format;
     2.8  
     2.9      for(i = 0; i < MAX_DISKS + MAX_SCSI_DISKS; i++)
    2.10          media_filename[i] = NULL;
    2.11 @@ -135,6 +136,8 @@ void xenstore_parse_domain_config(int hv
    2.12      }
    2.13          
    2.14      for (i = 0; i < num; i++) {
    2.15 +	format = NULL; /* don't know what the format is yet */
    2.16 +
    2.17          /* read the backend path */
    2.18          if (pasprintf(&buf, "%s/device/vbd/%s/backend", path, e[i]) == -1)
    2.19              continue;
    2.20 @@ -181,13 +184,20 @@ void xenstore_parse_domain_config(int hv
    2.21          drv = xs_read(xsh, XBT_NULL, buf, &len);
    2.22          if (drv == NULL)
    2.23              continue;
    2.24 -        /* Strip off blktap sub-type prefix aio: - QEMU can autodetect this */
    2.25 +        /* Obtain blktap sub-type prefix */
    2.26          if (!strcmp(drv, "tap") && params[0]) {
    2.27              char *offset = strchr(params, ':'); 
    2.28              if (!offset)
    2.29                  continue ;
    2.30 +	    free(drv);
    2.31 +	    drv = malloc(offset - params + 1);
    2.32 +	    memcpy(drv, params, offset - params);
    2.33 +	    drv[offset - params] = '\0';
    2.34 +	    if (!strcmp(drv, "aio"))
    2.35 +		/* qemu does aio anyway if it can */
    2.36 +		format = &bdrv_raw;
    2.37              memmove(params, offset+1, strlen(offset+1)+1 );
    2.38 -            fprintf(logfile, "Strip off blktap sub-type prefix to %s\n", params); 
    2.39 +            fprintf(logfile, "Strip off blktap sub-type prefix to %s (drv '%s')\n", params, drv); 
    2.40          }
    2.41          /* Prefix with /dev/ if needed */
    2.42          if (!strcmp(drv, "phy") && params[0] != '/') {
    2.43 @@ -195,6 +205,7 @@ void xenstore_parse_domain_config(int hv
    2.44              sprintf(newparams, "/dev/%s", params);
    2.45              free(params);
    2.46              params = newparams;
    2.47 +	    format = &bdrv_raw;
    2.48          }
    2.49  
    2.50          /* 
    2.51 @@ -240,8 +251,25 @@ void xenstore_parse_domain_config(int hv
    2.52  #endif
    2.53  
    2.54          if (params[0]) {
    2.55 -            if (bdrv_open(bs, params, 0 /* snapshot */) < 0)
    2.56 -                fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s'\n", buf, params);
    2.57 +	    if (!format) {
    2.58 +		if (!drv) {
    2.59 +		    fprintf(stderr, "qemu: type (image format) not specified for vbd '%s' or image '%s'\n", buf, params);
    2.60 +		    continue;
    2.61 +		}
    2.62 +		if (!strcmp(drv,"qcow")) {
    2.63 +		    /* autoguess qcow vs qcow2 */
    2.64 +		} else if (!strcmp(drv,"file")) {
    2.65 +		    format = &bdrv_raw;
    2.66 +		} else {
    2.67 +		    format = bdrv_find_format(drv);
    2.68 +		    if (!format) {
    2.69 +			fprintf(stderr, "qemu: type (image format) '%s' unknown for vbd '%s' or image '%s'\n", drv, buf, params);
    2.70 +			continue;
    2.71 +		    }
    2.72 +		}
    2.73 +	    }
    2.74 +            if (bdrv_open2(bs, params, 0 /* snapshot */, format) < 0)
    2.75 +                fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s' (drv '%s')\n", buf, params, drv ? drv : "?");
    2.76          }
    2.77      }
    2.78