Am 27.07.2010 20:25, schrieb Anthony Liguori:
> On 07/27/2010 12:43 PM, Anthony PERARD wrote:
>> Anthony Liguori wrote:
>>> On 07/27/2010 12:01 PM, Anthony PERARD wrote:
>>>> Anthony Liguori wrote:
>>>>> CVE-2008-2004 described a vulnerability in QEMU whereas a malicious
>>>>> user could
>>>>> trick the block probing code into accessing arbitrary files in a
>>>>> guest. To
>>>>> mitigate this, we added an explicit format parameter to -drive
>>>>> which disabling
>>>>> block probing.
>>>>>
>>>>> Fast forward to today, and the vast majority of users do not use
>>>>> this parameter.
>>>>> libvirt does not use this by default nor does virt-manager.
>>>>>
>>>>> Most users want block probing so we should try to make it safer.
>>>>>
>>>>> This patch adds some logic to the raw device which attempts to
>>>>> detect a write
>>>>> operation to the beginning of a raw device. If the first 4 bytes
>>>>> happen to
>>>>> match an image file that has a backing file that we support, it
>>>>> scrubs the
>>>>> signature to all zeros. If a user specifies an explicit format
>>>>> parameter, this
>>>>> behavior is disabled.
>>>>>
>>>>> I contend that while a legitimate guest could write such a
>>>>> signature to the
>>>>> header, we would behave incorrectly anyway upon the next invocation
>>>>> of QEMU.
>>>>> This simply changes the incorrect behavior to not involve a security
>>>>> vulnerability.
>>>>>
>>>>> I've tested this pretty extensively both in the positive and
>>>>> negative case. I'm
>>>>> not 100% confident in the block layer's ability to deal with zero
>>>>> sized writes
>>>>> particularly with respect to the aio functions so some additional
>>>>> eyes would be
>>>>> appreciated.
>>>>>
>>>>> Even in the case of a single sector write, we have to make sure to
>>>>> invoked the
>>>>> completion from a bottom half so just removing the zero sized write
>>>>> is not an
>>>>> option.
>>>>>
>>>>> Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
>>>>> ---
>>>>> v2 -> v3
>>>>> - add an assert to ensure the first iovec element is at least 512
>>>>> bytes
>>>>> v1 -> v2
>>>>> - be more paranoid about empty iovecs
>>>>> ---
>>>>> block.c | 4 ++
>>>>> block/raw.c | 130
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> block_int.h | 1 +
>>>>> 3 files changed, 135 insertions(+), 0 deletions(-)
>>>>
>>>>> static BlockDriverAIOCB *raw_aio_writev(BlockDriverState *bs,
>>>>> int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
>>>>> BlockDriverCompletionFunc *cb, void *opaque)
>>>>> {
>>>>> + const uint8_t *first_buf;
>>>>> + int first_buf_index = 0, i;
>>>>> +
>>>>> + /* This is probably being paranoid, but handle cases of zero size
>>>>> + vectors. */
>>>>> + for (i = 0; i < qiov->niov; i++) {
>>>>> + if (qiov->iov[i].iov_len) {
>>>>> + assert(qiov->iov[i].iov_len >= 512);
>>>>> + first_buf_index = i;
>>>>> + break;
>>>>> + }
>>>>> + }
>>>> Hi,
>>>>
>>>> I have try to do an installation of Windows XP SP2, with qemu fd2f659,
>>>> and the Assertion failed when windows begin to format the disk.
>>>>
>>>> The command line and the error message:
>>>> $ i386-softmmu/qemu -hda vm.img -cdrom winxpsp2.iso -boot dc
>>>> qemu: qemu/block/raw.c:130: raw_aio_writev: Assertion
>>>> `qiov->iov[i].iov_len >= 512' failed.
>>>>
>>>> And here, a little more information about the iov:
>>>> (gdb) p *qiov
>>>> $2 = {iov = 0x9106010, niov = 2, nalloc = 2, size = 512}
>>>> (gdb) p qiov->iov[0]
>>>> $3 = {iov_base = 0xaff3ce90, iov_len = 368}
>>>> (gdb) p qiov->iov[1]
>>>> $4 = {iov_base = 0xaff3f000, iov_len = 144}
>>>
>>> How can a single sector request be split between two iovs in QEMU?
>>> Are you carrying any patches in the version of QEMU that you're
>>> testing? Is this qemu-dm?
>>
>> Nop, I don't have any patch for this test. Is not qemu-dm.
>>
>>> To be clear, this is a discontiguous request. I'm looking at the core
>>> now in core.c and I don't see how an IDE disk can generate a request
>>> that looks like this.
>>>
>>> Can you provide a full stack trace?
>>
>> #0 0xb77dd424 in __kernel_vsyscall ()
>> #1 0xb7418640 in raise () from /lib/i686/cmov/libc.so.6
>> #2 0xb741a018 in abort () from /lib/i686/cmov/libc.so.6
>> #3 0xb74115be in __assert_fail () from /lib/i686/cmov/libc.so.6
>> #4 0x08074d30 in raw_aio_writev (bs=0xa5bcec0, sector_num=63,
>> qiov=0xa67cf14, nb_sectors=1, cb=0x81ae8c0 <dma_bdrv_cb>,
>> opaque=0xa67cee0) at /tmp/qemu-merge/block/raw.c:130
>> #5 0x0806d024 in bdrv_aio_writev (bs=0xa5bcec0, sector_num=63,
>> qiov=0xa67cf14, nb_sectors=1, cb=0x81ae8c0 <dma_bdrv_cb>,
>> opaque=0xa67cee0) at /tmp/qemu-merge/block.c:2004
>> #6 0x081aea78 in dma_bdrv_cb (opaque=0xa67cee0, ret=0) at
>> /tmp/qemu-merge/dma-helpers.c:120
>> #7 0x081aebc9 in dma_bdrv_io (bs=0xa5bcec0, sg=0xa61bd48,
>> sector_num=63, cb=0x81a9380 <ide_write_dma_cb>, opaque=0xa61c684,
>> is_write=1) at /tmp/qemu-merge/dma-helpers.c:163
>> #8 0x081a9484 in ide_write_dma_cb (opaque=0xa61c684, ret=0) at
>> /tmp/qemu-merge/hw/ide/core.c:748
>> #9 0x081a9eba in bmdma_cmd_writeb (opaque=0xa61c684, addr=49152,
>> val=1) at /tmp/qemu-merge/hw/ide/pci.c:51
>> #10 0x080a6b7b in cpu_outb (addr=6, val=<value optimized out>) at
>> /tmp/qemu-merge/ioport.c:80
>> #11 0xb5c95609 in ?? ()
>> #12 0x0000c000 in ?? ()
>> #13 0x00000001 in ?? ()
>> #14 0xff0a0000 in ?? ()
>> #15 0xbfa41448 in ?? ()
>> #16 0x00000000 in ?? ()
>
> Thanks. I see the problem. Working on a patch now.
>
> Regards,
>
> Anthony Liguori
Anthony, what happened with this one? I can't see a patch applied for
this and I just saw a similar report on the fedora-virt mailing list (no
backtrace yet, but the same assertion triggering).
Kevin
