Re: [Qemu-devel] [PATCH] ide: fix double free

[Gonglei (Arei)]

> -----Original Message-----
> From: Paolo Bonzini [mailto:address@hidden
> Sent: Wednesday, July 02, 2014 5:26 PM
> To: chenliang (T)
> Cc: Gonglei (Arei); address@hidden; address@hidden;
> address@hidden; Huangweidong (C)
> Subject: Re: [PATCH] ide: fix double free
> 
> Il 02/07/2014 11:24, ChenLiang ha scritto:
> > On 2014/7/2 17:04, Paolo Bonzini wrote:
> >
> >> This is definitely a heavyweight solution, and in fact the bug should
> >> not be there in the first place.  See dma_complete:
> >>
> >> static void dma_complete(DMAAIOCB *dbs, int ret)
> >> {
> >>     trace_dma_complete(dbs, ret, dbs->common.cb);
> >>
> >>     dma_bdrv_unmap(dbs);
> >>     if (dbs->common.cb) {
> >>         dbs->common.cb(dbs->common.opaque, ret);
> >>     }
> >>     qemu_iovec_destroy(&dbs->iov);
> >>     if (dbs->bh) {
> >>         qemu_bh_delete(dbs->bh);
> >>         dbs->bh = NULL;
> >>     }
> >>     if (!dbs->in_cancel) {
> >>         /* Requests may complete while dma_aio_cancel is in progress.
> In
> >>          * this case, the AIOCB should not be released because it is still
> >>          * referenced by dma_aio_cancel.  */
> >>         qemu_aio_release(dbs);
> >>     }
> >> }
> >>
> >> Perhaps something like this?
> >>
> >> diff --git a/dma-helpers.c b/dma-helpers.c
> >> index 53cbe92..21b70d12 100644
> >> --- a/dma-helpers.c
> >> +++ b/dma-helpers.c
> >> @@ -181,15 +181,15 @@ static void dma_aio_cancel(BlockDriverAIOCB
> *acb)
> >>
> >>      trace_dma_aio_cancel(dbs);
> >>
> >> +    dbs->in_cancel = true;
> >>      if (dbs->acb) {
> >>          BlockDriverAIOCB *acb = dbs->acb;
> >>          dbs->acb = NULL;
> >> -        dbs->in_cancel = true;
> >>          bdrv_aio_cancel(acb);
> >> -        dbs->in_cancel = false;
> >>      }
> >>      dbs->common.cb = NULL;
> >>      dma_complete(dbs, 0);
> >> +    qemu_aio_release(dbs);
> >>  }
> >>
> >>  static const AIOCBInfo dma_aiocb_info = {
> >>
> >>
> >> .
> >>
> >
> > acked, thanks
> 
> Did you test this? :)  What is the testcase?
> 
> Paolo

Tested-by: Gonglei <address@hidden>

Hi, Paolo. We have tested your above patch, and it works well for us.

The qemu command line list as bellow, which started by libvirt :

/home/qemu/x86_64-softmmu/qemu-system-x86_64 -name test -S -machine 
pc-i440fx-1.5,accel=kvm,usb=off -m 4096 -realtime mlock=off -smp 
16,sockets=16,cores=1,threads=1 -uuid abbbd27f-b83c-4c3b-8add-98df31eea0b7 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/test.monitor,server,nowait 
-mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device 
lsi,id=scsi0,bus=pci.0,addr=0x5 -drive 
file=/tmp/qemu-crash/linux_x86_64.iso,if=none,id=drive-ide0-0-0,readonly=on,format=raw,cache=none,aio=native
 -device ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 
-drive 
file=/home/test.img,if=none,id=drive-ide0-0-1,format=raw,cache=none,aio=native 
-device ide-hd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -netdev 
tap,fd=25,id=hostnet0 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:d9:3a:73,bus=pci.0,addr=0x3 
-device usb-tablet,id=input0 -vnc 0.0.0.0:0 -device 
cirrus-vga,id=video0,vgamem_mb=9,bus=pci.0,addr=0x2 

We setup a linux guest os on an IDE disk, named "test.img", when the guest os
Finishe the process of install, then I execute "reboot -f" in the VM 
immediately, 
And then will crash the qemu.

Best regards,
-Gonglei