Am 13.06.2016 um 13:30 hat Daniel P. Berrange geschrieben: > Back in the 2.3.0 release we declared qcow[2] encryption as > deprecated, warning people that it would be removed in a future > release. > > commit a1f688f4152e65260b94f37543521ceff8bfebe4 > Author: Markus Armbruster <[email protected]> > Date: Fri Mar 13 21:09:40 2015 +0100 > > block: Deprecate QCOW/QCOW2 encryption > > The code still exists today, but by a (happy?) accident we entirely > broke the ability to use qcow[2] encryption in the system emulators > in the 2.4.0 release due to > > commit 8336aafae1451d54c81dd2b187b45f7c45d2428e > Author: Daniel P. Berrange <[email protected]> > Date: Tue May 12 17:09:18 2015 +0100 > > qcow2/qcow: protect against uninitialized encryption key > > This commit was designed to prevent future coding bugs which > might cause QEMU to read/write data on an encrypted block > device in plain text mode before a decryption key is set. > > It turns out this preventative measure was a little too good, > because we already had a long standing bug where QEMU read > encrypted data in plain text mode during system emulator > startup, in order to guess disk geometry: > > Thread 10 (Thread 0x7fffd3fff700 (LWP 30373)): > #0 0x00007fffe90b1a28 in raise () at /lib64/libc.so.6 > #1 0x00007fffe90b362a in abort () at /lib64/libc.so.6 > #2 0x00007fffe90aa227 in __assert_fail_base () at /lib64/libc.so.6 > #3 0x00007fffe90aa2d2 in () at /lib64/libc.so.6 > #4 0x000055555587ae19 in qcow2_co_readv (bs=0x5555562accb0, sector_num=0, > remaining_sectors=1, qiov=0x7fffffffd260) at block/qcow2.c:1229 > #5 0x000055555589b60d in bdrv_aligned_preadv (bs=bs@entry=0x5555562accb0, > req=req@entry=0x7fffd3ffea50, offset=offset@entry=0, bytes=bytes@entry=512, > align=align@entry=512, qiov=qiov@entry=0x7fffffffd260, flags=0) at > block/io.c:908 > #6 0x000055555589b8bc in bdrv_co_do_preadv (bs=0x5555562accb0, offset=0, > bytes=512, qiov=0x7fffffffd260, flags=<optimized out>) at block/io.c:999 > #7 0x000055555589c375 in bdrv_rw_co_entry (opaque=0x7fffffffd210) at > block/io.c:544 > #8 0x000055555586933b in coroutine_thread (opaque=0x555557876310) at > coroutine-gthread.c:134 > #9 0x00007ffff64e1835 in g_thread_proxy (data=0x5555562b5590) at > gthread.c:778 > #10 0x00007ffff6bb760a in start_thread () at /lib64/libpthread.so.0 > #11 0x00007fffe917f59d in clone () at /lib64/libc.so.6 > > Thread 1 (Thread 0x7ffff7ecab40 (LWP 30343)): > #0 0x00007fffe91797a9 in syscall () at /lib64/libc.so.6 > #1 0x00007ffff64ff87f in g_cond_wait (cond=cond@entry=0x555555e085f0 > <coroutine_cond>, mutex=mutex@entry=0x555555e08600 <coroutine_lock>) at > gthread-posix.c:1397 > #2 0x00005555558692c3 in qemu_coroutine_switch (co=<optimized out>) at > coroutine-gthread.c:117 > #3 0x00005555558692c3 in qemu_coroutine_switch (from_=0x5555562b5e30, > to_=to_@entry=0x555557876310, action=action@entry=COROUTINE_ENTER) at > coroutine-gthread.c:175 > #4 0x0000555555868a90 in qemu_coroutine_enter (co=0x555557876310, > opaque=0x0) at qemu-coroutine.c:116 > #5 0x0000555555859b84 in thread_pool_completion_bh (opaque=0x7fffd40010e0) > at thread-pool.c:187 > #6 0x0000555555859514 in aio_bh_poll (ctx=ctx@entry=0x5555562953b0) at > async.c:85 > #7 0x0000555555864d10 in aio_dispatch (ctx=ctx@entry=0x5555562953b0) at > aio-posix.c:135 > #8 0x0000555555864f75 in aio_poll (ctx=ctx@entry=0x5555562953b0, > blocking=blocking@entry=true) at aio-posix.c:291 > #9 0x000055555589c40d in bdrv_prwv_co (bs=bs@entry=0x5555562accb0, > offset=offset@entry=0, qiov=qiov@entry=0x7fffffffd260, > is_write=is_write@entry=false, flags=flags@entry=(unknown: 0)) at > block/io.c:591 > #10 0x000055555589c503 in bdrv_rw_co (bs=bs@entry=0x5555562accb0, > sector_num=sector_num@entry=0, buf=buf@entry=0x7fffffffd2e0 "\321,", > nb_sectors=nb_sectors@entry=21845, is_write=is_write@entry=false, > flags=flags@entry=(unknown: 0)) at block/io.c:614 > #11 0x000055555589c562 in bdrv_read_unthrottled (nb_sectors=21845, > buf=0x7fffffffd2e0 "\321,", sector_num=0, bs=0x5555562accb0) at block/io.c:622 > #12 0x000055555589c562 in bdrv_read_unthrottled (bs=0x5555562accb0, > sector_num=sector_num@entry=0, buf=buf@entry=0x7fffffffd2e0 "\321,", > nb_sectors=nb_sectors@entry=21845) at block/io.c:634 > nb_sectors@entry=1) at block/block-backend.c:504 > #14 0x0000555555752e9f in guess_disk_lchs (blk=blk@entry=0x5555562a5290, > pcylinders=pcylinders@entry=0x7fffffffd52c, > pheads=pheads@entry=0x7fffffffd530, psectors=psectors@entry=0x7fffffffd534) > at hw/block/hd-geometry.c:68 > #15 0x0000555555752ff7 in hd_geometry_guess (blk=0x5555562a5290, > pcyls=pcyls@entry=0x555557875d1c, pheads=pheads@entry=0x555557875d20, > psecs=psecs@entry=0x555557875d24, ptrans=ptrans@entry=0x555557875d28) at > hw/block/hd-geometry.c:133 > #16 0x0000555555752b87 in blkconf_geometry (conf=conf@entry=0x555557875d00, > ptrans=ptrans@entry=0x555557875d28, cyls_max=cyls_max@entry=65536, > heads_max=heads_max@entry=16, secs_max=secs_max@entry=255, > errp=errp@entry=0x7fffffffd5e0) at hw/block/block.c:71 > #17 0x0000555555799bc4 in ide_dev_initfn (dev=0x555557875c80, kind=IDE_HD) > at hw/ide/qdev.c:174 > #18 0x0000555555768394 in device_realize (dev=0x555557875c80, > errp=0x7fffffffd640) at hw/core/qdev.c:247 > #19 0x0000555555769a81 in device_set_realized (obj=0x555557875c80, > value=<optimized out>, errp=0x7fffffffd730) at hw/core/qdev.c:1058 > #20 0x00005555558240ce in property_set_bool (obj=0x555557875c80, > v=<optimized out>, opaque=0x555557875de0, name=<optimized out>, > errp=0x7fffffffd730) > at qom/object.c:1514 > #21 0x0000555555826c87 in object_property_set_qobject > (obj=obj@entry=0x555557875c80, value=value@entry=0x55555784bcb0, > name=name@entry=0x55555591cb3d "realized", errp=errp@entry=0x7fffffffd730) at > qom/qom-qobject.c:24 > #22 0x0000555555825760 in object_property_set_bool > (obj=obj@entry=0x555557875c80, value=value@entry=true, > name=name@entry=0x55555591cb3d "realized", errp=errp@entry=0x7fffffffd730) at > qom/object.c:905 > #23 0x000055555576897b in qdev_init_nofail (dev=dev@entry=0x555557875c80) > at hw/core/qdev.c:380 > #24 0x0000555555799ead in ide_create_drive (bus=bus@entry=0x555557629630, > unit=unit@entry=0, drive=0x5555562b77e0) at hw/ide/qdev.c:122 > #25 0x000055555579a746 in pci_ide_create_devs > (dev=dev@entry=0x555557628db0, hd_table=hd_table@entry=0x7fffffffd830) at > hw/ide/pci.c:440 > #26 0x000055555579b165 in pci_piix3_ide_init (bus=<optimized out>, > hd_table=0x7fffffffd830, devfn=<optimized out>) at hw/ide/piix.c:218 > #27 0x000055555568ca55 in pc_init1 (machine=0x5555562960a0, pci_enabled=1, > kvmclock_enabled=<optimized out>) at > /home/berrange/src/virt/qemu/hw/i386/pc_piix.c:256 > #28 0x0000555555603ab2 in main (argc=<optimized out>, argv=<optimized out>, > envp=<optimized out>) at vl.c:4249 > > So the safety net is correctly preventing QEMU reading cipher > text as if it were plain text, during startup and aborting QEMU > to avoid bad usage of this data. > > For added fun this bug only happens if the encrypted qcow2 > file happens to have data written to the first cluster, > otherwise the cluster won't be allocated and so qcow2 would > not try the decryption routines at all, just return all 0's. > > That no one even noticed, let alone reported, this bug that > has shipped in 2.4.0, 2.5.0 and 2.6.0 shows that the number > of actual users of qcow2 is approximately zero.
s/qcow2/encrypted qcow2/, I'd say. > So rather than fix the crash, and backport it to stable > releases, just go ahead with what we have warned users about > and disable any use of qcow2 encryption in the system > emulators. qemu-img/qemu-io/qemu-nbd are still able to access > qcow2 encrypted images for the sake of data conversion. > > In the future, qcow2 will gain support for the alternative > luks format, but when this happens it'll be using the > '-object secret' infrastructure for gettings keys, which > avoids this problematic scenario entirely. > > Signed-off-by: Daniel P. Berrange <[email protected]> Apart from the commit message comments above and from Eric, this looks good to me. I'll give a little more time for review before merging this. Kevin
