# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1210688387 -3600
# Node ID 80730d294e51e39a7f8f58708d1de2f735001392
# Parent fd285b18158e8bc355ac036cf9d305d06bbfbce3
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>
xen-unstable changeset: 17606:e3be00bd6aa963aca563692c271af762f9380ba0
xen-unstable date: Mon May 12 10:09:12 2008 +0100
---
tools/ioemu/block.c | 2 +-
tools/ioemu/xenstore.c | 37 ++++++++++++++++++++++++++++++++-----
2 files changed, 33 insertions(+), 6 deletions(-)
diff -r fd285b18158e -r 80730d294e51 tools/ioemu/block.c
--- a/tools/ioemu/block.c Tue May 13 15:16:59 2008 +0100
+++ b/tools/ioemu/block.c Tue May 13 15:19:47 2008 +0100
@@ -250,7 +250,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 fd285b18158e -r 80730d294e51 tools/ioemu/xenstore.c
--- a/tools/ioemu/xenstore.c Tue May 13 15:16:59 2008 +0100
+++ b/tools/ioemu/xenstore.c Tue May 13 15:19:47 2008 +0100
@@ -86,6 +86,7 @@ void xenstore_parse_domain_config(int do
int i, is_scsi, is_hdN = 0;
unsigned int len, num, hd_index;
BlockDriverState *bs;
+ BlockDriver *format;
for(i = 0; i < MAX_DISKS + MAX_SCSI_DISKS; i++)
media_filename[i] = NULL;
@@ -131,6 +132,8 @@ void xenstore_parse_domain_config(int do
}
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;
@@ -177,13 +180,20 @@ void xenstore_parse_domain_config(int do
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] != '/') {
@@ -191,6 +201,7 @@ void xenstore_parse_domain_config(int do
sprintf(newparams, "/dev/%s", params);
free(params);
params = newparams;
+ format = &bdrv_raw;
}
/*
@@ -227,9 +238,25 @@ void xenstore_parse_domain_config(int do
/* open device now if media present */
if (params[0]) {
- if (bdrv_open(bs, params, 0 /* snapshot */) < 0)
- fprintf(stderr, "qemu: could not open hard disk image '%s'\n",
- 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-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|