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

[ChenLiang <chenliang88@huawei.com>]

On 2014/7/2 19:40, Paolo Bonzini wrote:

> Il 02/07/2014 13:33, ChenLiang ha scritto:
>> On 2014/7/2 18:16, Paolo Bonzini wrote:
>>
>>> Il 02/07/2014 11:46, Gonglei (Arei) ha scritto:
>>>> Hi, Paolo. We have tested your above patch, and it works well for us.
>>>
>>> I'm still not sure where the fix is.  I jotted the patch quickly, but I'd rather understand it better before submitting it.  Here is it again:
>>>
>>> --- 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);
>>>  }
>>>
>>
>>
>> Hmm, dbs->in_cancel will be true always. Although this will avoid freeing dbs by dma_comlete.
>> But it maybe a mistake.
> 
> This was on purpose; I'm doing the free myself in dma_aio_cancel, so I wanted to avoid the qemu_aio_release from dma_complete.  This was in case of a recursive call to dma_complete.  But I don't see how that recursive call could happen outside the "if (dbs->acb)"; and inside the "if" the protection is there already.
> 
> Can you gather the backtraces for _both_ calls to qemu_aio_release, rather than just the second?
> 
> With what guest are you encountering the bug?
> 
> Paolo
> 
> 
> .
> 

(gdb) bt
#0  qemu_aio_release (p=0x7f44788d1290) at block.c:4260
#1  0x00007f4477494e5e in dma_complete (dbs=0x7f44788d1290, ret=0) at dma-helpers.c:135
#2  0x00007f44774952c2 in dma_aio_cancel (acb=0x7f44788d1290) at dma-helpers.c:195
#3  0x00007f447744825b in bdrv_aio_cancel (acb=0x7f44788d1290) at block.c:3848
#4  0x00007f4477513911 in ide_bus_reset (bus=0x7f44785f1bd8) at hw/ide/core.c:1957
#5  0x00007f4477516b3c in piix3_reset (opaque=0x7f44785f1530) at hw/ide/piix.c:113
#6  0x00007f4477647b9f in qemu_devices_reset () at vl.c:2131
#7  0x00007f4477647c0f in qemu_system_reset (report=true) at vl.c:2140
#8  0x00007f4477648127 in main_loop_should_exit () at vl.c:2274
#9  0x00007f447764823a in main_loop () at vl.c:2323
#10 0x00007f447764f6da in main (argc=57, argv=0x7fff5d194378, envp=0x7fff5d194548) at vl.c:4803

Chenliang


is