Thanks to everyone for their comments. It turns out that the
file:/path/to/image
syntax doesn't work properly at the moment unless the image is a raw
image: although qemu-dm will treat it as a cow image, if it is in a
format it understands, blktap will not. This means that the guest
would see the processed cow version via the emulated IDE controller
but the raw cow differences file, header and all, via the PV block
interface.
So it is fine for us to make file:/path/to/image always expect raw
images. Non-raw images are done with tap:qcow:/path/to/image
and that currently works.
Below is a patch, with suggested commit message.
The resulting code in xenstore.c is sadly not very simple. This is
because it is untangling a mess made (entirely pointlessly) by xend,
as well as translating between the different naming schemes used by
xenstore and qemu for image formats. I hope to remove this block
driver special case machinery from qemu as part of or shortly after I
complete my merge with qemu upstream.
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@xxxxxxxxxxxxx>
Ian.
diff -r c99a88623eda tools/ioemu/block.c
--- a/tools/ioemu/block.c Thu May 08 14:33:31 2008 +0100
+++ b/tools/ioemu/block.c Fri May 09 15:31:46 2008 +0100
@@ -254,7 +254,7 @@ static BlockDriver *find_protocol(const
#endif
p = strchr(filename, ':');
if (!p)
- return &bdrv_raw;
+ return NULL; /* do not ever guess raw, it is a security problem! */
len = p - filename;
if (len > sizeof(protocol) - 1)
len = sizeof(protocol) - 1;
diff -r c99a88623eda tools/ioemu/xenstore.c
--- a/tools/ioemu/xenstore.c Thu May 08 14:33:31 2008 +0100
+++ b/tools/ioemu/xenstore.c Fri May 09 16:32:43 2008 +0100
@@ -90,6 +90,7 @@ void xenstore_parse_domain_config(int hv
int i, is_scsi, is_hdN = 0;
unsigned int len, num, hd_index, pci_devid = 0;
BlockDriverState *bs;
+ BlockDriver *format;
for(i = 0; i < MAX_DISKS + MAX_SCSI_DISKS; i++)
media_filename[i] = NULL;
@@ -135,6 +136,8 @@ void xenstore_parse_domain_config(int hv
}
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;
@@ -181,13 +184,20 @@ void xenstore_parse_domain_config(int hv
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] != '/') {
@@ -195,6 +205,7 @@ void xenstore_parse_domain_config(int hv
sprintf(newparams, "/dev/%s", params);
free(params);
params = newparams;
+ format = &bdrv_raw;
}
/*
@@ -240,8 +251,25 @@ void xenstore_parse_domain_config(int hv
#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 {
+ 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')\n", buf, params, drv ? drv : "?");
}
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home