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?
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?
Regards,
Anthony Liguori
Without the assert, the install work fine.
Regards,
