[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] block: drop support for using qcow[2] encryp
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2] block: drop support for using qcow[2] encryption with system emulators |
Date: |
Mon, 13 Jun 2016 17:36:41 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> 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 <address@hidden>
> 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 (address@hidden,
> address@hidden, address@hidden, address@hidden, address@hidden,
> address@hidden, 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 (address@hidden <coroutine_cond>,
> address@hidden <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,
> address@hidden, address@hidden) 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 (address@hidden) at async.c:85
> #7 0x0000555555864d10 in aio_dispatch (address@hidden) at aio-posix.c:135
> #8 0x0000555555864f75 in aio_poll (address@hidden, address@hidden) at
> aio-posix.c:291
> #9 0x000055555589c40d in bdrv_prwv_co (address@hidden, address@hidden,
> address@hidden, address@hidden, address@hidden(unknown: 0)) at block/io.c:591
> #10 0x000055555589c503 in bdrv_rw_co (address@hidden, address@hidden,
> address@hidden "\321,", address@hidden, address@hidden,
> address@hidden(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,
> address@hidden, address@hidden "\321,", address@hidden) at block/io.c:634
> address@hidden) at block/block-backend.c:504
> #14 0x0000555555752e9f in guess_disk_lchs (address@hidden, address@hidden,
> address@hidden, address@hidden) at hw/block/hd-geometry.c:68
> #15 0x0000555555752ff7 in hd_geometry_guess (blk=0x5555562a5290,
> address@hidden, address@hidden, address@hidden, address@hidden) at
> hw/block/hd-geometry.c:133
> #16 0x0000555555752b87 in blkconf_geometry (address@hidden, address@hidden,
> address@hidden, address@hidden, address@hidden, address@hidden) 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 (address@hidden,
> address@hidden, address@hidden "realized", address@hidden) at
> qom/qom-qobject.c:24
> #22 0x0000555555825760 in object_property_set_bool (address@hidden,
> address@hidden, address@hidden "realized", address@hidden) at qom/object.c:905
> #23 0x000055555576897b in qdev_init_nofail (address@hidden) at
> hw/core/qdev.c:380
> #24 0x0000555555799ead in ide_create_drive (address@hidden, address@hidden,
> drive=0x5555562b77e0) at hw/ide/qdev.c:122
> #25 0x000055555579a746 in pci_ide_create_devs (address@hidden,
> address@hidden) 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 <address@hidden>
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