Description of problem: Chris Wright has reported the following qemu removable (usb, floppy) media related problem (pointed out by Markus Armbruster): Previous commit didn't handle removable media or USB (thanks to Markus for noting this). This patch adds a cmdline option for USB to allow admin to specify format type. To avoid changing exists semantics a new option -usbdevice diskformat: is added (ugly name). This is valid from both command line and monitor interface. Because of the comma delimiter, admin must use ',,' just as in -drive file=filename. The patch also allows specifying image format when changing removable media. It is an optional argument to the monitor command "change," so there is no change to existing semantics. Longer term it'd be better to provide some safe defaults. Additional info: This issue is related with qemu block format auto-detection vulnerability (CVE-2008-2004).
Proposed patch from Chris Wright: --- hw/usb-msd.c | 12 ++++++++++-- hw/usb.h | 2 +- monitor.c | 20 ++++++++++++++------ qemu-doc.texi | 4 ++++ vl.c | 13 ++++++++++++- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/hw/usb-msd.c b/hw/usb-msd.c index 01d492d..3c3f39a 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -513,17 +513,25 @@ static void usb_msd_handle_destroy(USBDevice *dev) qemu_free(s); } -USBDevice *usb_msd_init(const char *filename) +USBDevice *usb_msd_init(const char *filename, const char *fmt) { MSDState *s; BlockDriverState *bdrv; + BlockDriver *drv = NULL; + if (fmt) { + drv = bdrv_find_format(fmt); + if (!drv) { + fprintf(stderr, "%s: '%s' invalid format\n", __func__, fmt); + return NULL; + } + } s = qemu_mallocz(sizeof(MSDState)); if (!s) return NULL; bdrv = bdrv_new("usb"); - if (bdrv_open(bdrv, filename, 0) < 0) + if (bdrv_open2(bdrv, filename, 0, drv) < 0) goto fail; if (qemu_key_check(bdrv, filename)) goto fail; diff --git a/hw/usb.h b/hw/usb.h index 9b759d4..a2cb313 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -217,7 +217,7 @@ USBDevice *usb_tablet_init(void); USBDevice *usb_keyboard_init(void); /* usb-msd.c */ -USBDevice *usb_msd_init(const char *filename); +USBDevice *usb_msd_init(const char *filename, const char *fmt); /* usb-wacom.c */ USBDevice *usb_wacom_init(void); diff --git a/monitor.c b/monitor.c index 236b827..8d79d5c 100644 --- a/monitor.c +++ b/monitor.c @@ -405,18 +405,26 @@ static void do_eject(int force, const char *filename) eject_device(bs, force); } -static void do_change_block(const char *device, const char *filename) +static void do_change_block(const char *device, const char *filename, const char *fmt) { BlockDriverState *bs; + BlockDriver *drv = NULL; bs = bdrv_find(device); if (!bs) { term_printf("device not found\n"); return; } + if (fmt) { + drv = bdrv_find_format(fmt); + if (!drv) { + term_printf("invalid format %s\n", fmt); + return; + } + } if (eject_device(bs, 0) < 0) return; - bdrv_open(bs, filename, 0); + bdrv_open2(bs, filename, 0, drv); qemu_key_check(bs, filename); } @@ -435,12 +443,12 @@ static void do_change_vnc(const char *target) } } -static void do_change(const char *device, const char *target) +static void do_change(const char *device, const char *target, const char *fmt) { if (strcmp(device, "vnc") == 0) { do_change_vnc(target); } else { - do_change_block(device, target); + do_change_block(device, target, fmt); } } @@ -1322,8 +1330,8 @@ static term_cmd_t term_cmds[] = { "", "quit the emulator" }, { "eject", "-fB", do_eject, "[-f] device", "eject a removable medium (use -f to force it)" }, - { "change", "BF", do_change, - "device filename", "change a removable medium" }, + { "change", "BFs?", do_change, + "device filename [format]", "change a removable medium, optional format" }, { "screendump", "F", do_screen_dump, "filename", "save screen into PPM image 'filename'" }, { "logfile", "s", do_logfile, diff --git a/qemu-doc.texi b/qemu-doc.texi index 4a48f43..34743f2 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -547,6 +547,10 @@ mouse. Also overrides the PS/2 mouse emulation when activated. @item disk:file Mass storage device based on file +@item diskformat:file,format=@var{format} +Mass storage device based on file with specified image @var{format}. +See -drive file= and format= for more information. + @item host:bus.addr Pass through the host device identified by bus.addr (Linux only). diff --git a/vl.c b/vl.c index ea8c63c..febecef 100644 --- a/vl.c +++ b/vl.c @@ -5351,7 +5351,18 @@ static int usb_device_add(const char *devname) } else if (!strcmp(devname, "keyboard")) { dev = usb_keyboard_init(); } else if (strstart(devname, "disk:", &p)) { - dev = usb_msd_init(p); + dev = usb_msd_init(p, NULL); + } else if (strstart(devname, "diskformat:", &p)) { + char file[1024]; + char buf[128]; + char *fmt = NULL; + p = get_opt_value(file, sizeof(file), p); + if (*p == ',') { + p++; + if (get_param_value(buf, sizeof(buf), "format", p)) + fmt = buf; + } + dev = usb_msd_init(file, fmt); } else if (!strcmp(devname, "wacom-tablet")) { dev = usb_wacom_init(); } else if (strstart(devname, "serial:", &p)) {
Yet more details from Chris: Unfortunately, this doesn't care for removable media. (qemu) info block floppy0: type=floppy removable=1 locked=0 file=/mnt/disk2 ro=0 drv=raw guest writes malicious qcow header to its /dev/floppy (qemu) change floppy0 /mnt/disk2 (qemu) info block floppy0: type=floppy removable=1 locked=0 file=/mnt/disk2 backing_file=/etc/passwd ro=0 drv=qcow2 Nor does it care for usb disk devices. I've got a patch for these, although it's getting a bit ugly. Below is against qemu svn tip.
There's no need to fix QEMU monitor commands for RHEL-5, because we disabled the monitor (bug 230295). We need to fix -usbdevice, because that is accessible through /etc/xen/DOMNAME key usbdevice.
Issue is public already, opening bug.
The dependant bugs were closed out long ago, closing out this tracker. Chris Lalancette