Re: [Qemu-devel] [PATCH 3/5] push down vector linearization to posix-aio-compat.c

[Anthony Liguori <anthony@codemonkey.ws>]

Christoph Hellwig wrote:
> On Sun, Mar 29, 2009 at 04:01:26PM -0500, Anthony Liguori wrote:
>   
>> This bug is not limited to win32 though.  It has to do with having a 
>> backend disk format that does not provide a proper aio implement (which 
>> suggests your new brv_aio_{readv,writev}_em at fault).  You can 
>> reproduce on a normal Linux build by converting an existing image to a 
>> format like vmdk and then testing with that.  It's not 100% reliable but 
>> 9 times out of 10 I don't get past the grub loader with this patch applied.
>>     
>
> This bug?  I did test cow, vmdk and vpc and can't reproduce it.  By
> inspection I can find a use after free in bdrv_aio_bh_cb, though
>   

I assume you mean this.  I've confirmed that with this fix, it now works 
with win32 and vmdk on Linux.  It doesn't get triggered unless you're 
bouncing in block.c which won't happen unless you have a driver that 
doesn't support the aio functions.  That's why I couldn't reproduce it 
without vmdk on Linux.

diff --git a/block.c b/block.c
index b41e421..49c38c1 100644
--- a/block.c
+++ b/block.c
@@ -1334,12 +1334,12 @@ static void bdrv_aio_bh_cb(void *opaque)
 {
     BlockDriverAIOCBSync *acb = opaque;
 
-    qemu_vfree(acb->bounce);
-
     if (!acb->is_write)
         qemu_iovec_from_buffer(acb->qiov, acb->bounce, acb->qiov->size);
     acb->common.cb(acb->common.opaque, acb->ret);
 
+    qemu_vfree(acb->bounce);
+
     qemu_aio_release(acb);
 }

> Will repost the whole series, the win32 aio removal and some
> recent scsi-disk commits caused a whole lot of rejects..
>   

Just two and I've fixed them locally but I can wait for you to resubmit 
if you'd like.

Regards,

Anthony Liguori