From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56318) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WORa4-0000yF-Rt for qemu-devel@nongnu.org; Fri, 14 Mar 2014 08:51:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WORZx-0006z9-KV for qemu-devel@nongnu.org; Fri, 14 Mar 2014 08:51:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37223) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WORZx-0006yh-Ck for qemu-devel@nongnu.org; Fri, 14 Mar 2014 08:51:13 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2ECpBIi032457 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Mar 2014 08:51:12 -0400 Message-ID: <5322FB3B.2020507@redhat.com> Date: Fri, 14 Mar 2014 13:51:07 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1394785368-24437-1-git-send-email-armbru@redhat.com> In-Reply-To: <1394785368-24437-1-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, stefanha@redhat.com Il 14/03/2014 09:22, Markus Armbruster ha scritto: > Opening an encrypted image takes an additional step: setting the key. > Between open and the key set, the image must not be used. > > We have some protection against accidental use in place: you can't > unpause a guest while we're missing keys. You can, however, hot-plug > block devices lacking keys into a running guest just fine, or insert > media lacking keys. In the latter case, notifying the guest of the > insert is delayed until the key is set, which may suffice to protect > at least some guests in common usage. > > This patch makes the protection apply in more cases, in a rather > heavy-handed way: it doesn't let you open encrypted images unless > we're in a paused state. > > It doesn't extend the protection to users other than the guest (block > jobs?). Use of runstate_check() from block.c is disgusting. Best I > can do right now. > > Signed-off-by: Markus Armbruster > --- > v2: > * Don't break -incoming [Paolo] > * Update qemu-iotests/087 [Fam] Reviewed-by: Paolo Bonzini > block.c | 9 ++++++++- > stubs/Makefile.objs | 1 + > stubs/runstate-check.c | 6 ++++++ > tests/qemu-iotests/087 | 17 +++++++++++++++++ > tests/qemu-iotests/087.out | 11 ++++++++++- > 5 files changed, 42 insertions(+), 2 deletions(-) > create mode 100644 stubs/runstate-check.c > > diff --git a/block.c b/block.c > index fae50c9..53f5b44 100644 > --- a/block.c > +++ b/block.c > @@ -1388,12 +1388,19 @@ done: > ret = -EINVAL; > goto close_and_fail; > } > - QDECREF(options); > > if (!bdrv_key_required(bs)) { > bdrv_dev_change_media_cb(bs, true); > + } else if (!runstate_check(RUN_STATE_PRELAUNCH) > + && !runstate_check(RUN_STATE_INMIGRATE) > + && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */ > + error_setg(errp, > + "Guest must be stopped for opening of encrypted image"); > + ret = -EBUSY; > + goto close_and_fail; > } > > + QDECREF(options); > *pbs = bs; > return 0; > > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > index 59c5a54..5ed1d38 100644 > --- a/stubs/Makefile.objs > +++ b/stubs/Makefile.objs > @@ -20,6 +20,7 @@ stub-obj-y += mon-set-error.o > stub-obj-y += pci-drive-hot-add.o > stub-obj-y += qtest.o > stub-obj-y += reset.o > +stub-obj-y += runstate-check.o > stub-obj-y += set-fd-handler.o > stub-obj-y += slirp.o > stub-obj-y += sysbus.o > diff --git a/stubs/runstate-check.c b/stubs/runstate-check.c > new file mode 100644 > index 0000000..bd2e375 > --- /dev/null > +++ b/stubs/runstate-check.c > @@ -0,0 +1,6 @@ > +#include "sysemu/sysemu.h" > + > +bool runstate_check(RunState state) > +{ > + return state == RUN_STATE_PRELAUNCH; > +} > diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087 > index 53b6c43..a38bb70 100755 > --- a/tests/qemu-iotests/087 > +++ b/tests/qemu-iotests/087 > @@ -99,6 +99,23 @@ echo === Encrypted image === > echo > > _make_test_img -o encryption=on $size > +run_qemu -S < +{ "execute": "qmp_capabilities" } > +{ "execute": "blockdev-add", > + "arguments": { > + "options": { > + "driver": "$IMGFMT", > + "id": "disk", > + "file": { > + "driver": "file", > + "filename": "$TEST_IMG" > + } > + } > + } > + } > +{ "execute": "quit" } > +EOF > + > run_qemu < { "execute": "qmp_capabilities" } > { "execute": "blockdev-add", > diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out > index b871032..e65dcdf 100644 > --- a/tests/qemu-iotests/087.out > +++ b/tests/qemu-iotests/087.out > @@ -28,7 +28,7 @@ QMP_VERSION > === Encrypted image === > > Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on > -Testing: > +Testing: -S > QMP_VERSION > {"return": {}} > {"error": {"class": "GenericError", "desc": "blockdev-add doesn't support encrypted devices"}} > @@ -37,4 +37,13 @@ QMP_VERSION > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} > > +Testing: > +QMP_VERSION > +{"return": {}} > +{"error": {"class": "GenericError", "desc": "could not open disk image disk: Guest must be stopped for opening of encrypted image"}} > +{"return": {}} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} > + > *** done >