[libvirt] PATCH] Stop double free

[Jim Fehlig]

Chris Lalancette wrote:
> Mark Hamzy wrote:
>   
>> The stack trace is as follows:
>>
>> Program received signal SIGABRT, Aborted.
>> 0x00000035ad830265 in raise () from /lib64/libc.so.6
>> (gdb) bt
>> #0 0x00000035ad830265 in raise () from /lib64/libc.so.6
>> #1 0x00000035ad831d10 in abort () from /lib64/libc.so.6
>> #2 0x00000035ad86a84b in __libc_message () from /lib64/libc.so.6
>> #3 0x00000035ad8722ef in _int_free () from /lib64/libc.so.6
>> #4 0x00000035ad87273b in free () from /lib64/libc.so.6
>> #5 0x0000000000406771 in vshDeinit (ctl=0x7fffd35d35e0) at virsh.c:8244
>> #6 0x00000000004069a5 in vshError (ctl=0x7fffd35d35e0, doexit=<value
>> optimized out>, format=0x414f66 "%s") at virsh.c:7861
>> #7 0x00000000004067c4 in vshDeinit (ctl=0x7fffd35d35e0) at virsh.c:8248
>> #8 0x000000000041335e in main (argc=3, argv=0x7fffd35d3748) at virsh.c:8493
>>
>> I am trying to run libvirt-0.7.1-0.1.git3ef2e05.fc12.src.rpm on RHEL5.4.
>>
>> vshDeinit gets called twice, so ctl->name is freed twice.
>>
>> How about this patch then?
>>     
>
> Ah, I see now.  Your patch is a workaround.  The real problem is that vshDeinit
> is re-entering itself through:
>
> vshDeinit()->vshError()->vshDeinit()
>
> While your patch would fix the problem, I'm not sure it's a good long-term
> solution.  Other differences might come up in the future, and trying to worry
> about vshDeinit being re-entrant is probably not worth the effort.  (Indeed, it
> looks like there were earlier attempts to avoid this, but things have changed
> since then, breaking the workaround).  I think we should make it so that
> vshDeinit() does not try to re-enter itself.  At the moment I don't have a
> patch, but I would look at either splitting vshError() into vshPrintError() and
> vshError(), or just doing a couple of fprintf()'s directly in vshDeinit() and
> not calling vshError() at all (with a comment explaining why).
>   

I'm seeing this problem in 0.7.1 as well when explicitly providing a
connection URI, e.g. 'virsh -c ...'.  I cooked up a patch based on your
latter suggestion but changed it to the attached after chatting with
danpb on IRC.  This approach removes the doexit parameter and
vshDeinit() call from vshError() altogether, requiring callers of
vshError() to exit if they so desire.

Regards,
Jim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vshDeinit.patch
Type: text/x-patch
Size: 75872 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090928/a8ee62b7/attachment-0001.bin>