From 7afcc1f9bae3e857834a3bb8247be101e2354998 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 12 Mar 2015 17:26:49 +0100 Subject: [PATCH] usb/dev-storage: Fix QMP device_add missing encryption key failure When the image is encrypted, QMP device_add creates the device, defers actually attaching it to when the key becomes available, then returns an error. This is wrong. device_add must either create the device and succeed, or do nothing and fail. The bug is in usb_msd_realize_storage(). It posts an error with qerror_report_err(), and returns success. Device realization relies on the return value, and completes. The QMP monitor, however, relies on the posted error, and sends it in an error reply. Reproducer: $ qemu-system-x86_64 -nodefaults -display none -usb -qmp stdio -drive if=none,id=foo,file=geheim.qcow2 {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}} { "execute": "qmp_capabilities" } {"return": {}} { "execute": "device_add", "arguments": { "driver": "usb-storage", "id": "bar", "drive": "foo" } } {"error": {"class": "DeviceEncrypted", "desc": "'foo' (geheim.qcow2) is encrypted"}} Even though we got an error back, the device got created just fine. To demonstrate, let's unplug it again: {"execute":"device_del","arguments": { "id": "bar" } } {"timestamp": {"seconds": 1426003440, "microseconds": 237181}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/bar/bar.0/legacy[0]"}} {"timestamp": {"seconds": 1426003440, "microseconds": 238231}, "event": "DEVICE_DELETED", "data": {"device": "bar", "path": "/machine/peripheral/bar"}} {"return": {}} Fix by making usb_msd_realize_storage() fail properly. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Gerd Hoffmann --- hw/usb/dev-storage.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index f47c8561ef..f50bcb83e2 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -610,6 +610,23 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp) return; } + bdrv_add_key(blk_bs(blk), NULL, &err); + if (err) { + if (monitor_cur_is_qmp()) { + error_propagate(errp, err); + return; + } + error_free(err); + err = NULL; + if (cur_mon) { + monitor_read_bdrv_key_start(cur_mon, blk_bs(blk), + usb_msd_password_cb, s); + s->dev.auto_attach = 0; + } else { + autostart = 0; + } + } + blkconf_serial(&s->conf, &dev->serial); blkconf_blocksizes(&s->conf); @@ -638,25 +655,6 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp) } usb_msd_handle_reset(dev); s->scsi_dev = scsi_dev; - - if (bdrv_key_required(blk_bs(blk))) { - if (cur_mon) { - bdrv_add_key(blk_bs(blk), NULL, &err); - if (!err) { - usb_msd_password_cb(s, 0); - } else if (monitor_cur_is_qmp()) { - qerror_report_err(err); - error_free(err); - } else { - error_free(err); - monitor_read_bdrv_key_start(cur_mon, blk_bs(blk), - usb_msd_password_cb, s); - } - s->dev.auto_attach = 0; - } else { - autostart = 0; - } - } } static void usb_msd_realize_bot(USBDevice *dev, Error **errp) -- 2.11.0