[libvirt] [PATCH]daemon: Fix a crash during virNetlinkEventServiceStopAll

[Haitaoliu]

Thank you for reminding me of that comment.  Please ignore this patch, 
for the comments are inaccurate.

I will send it again with  steps to reproduce  the bug.

On 2019/6/11 09:39, Laine Stump wrote:
> On 6/10/19 2:15 AM, Liu Haitao wrote:
>> The virNetlinkEventServiceStopAll() should be executed behind 
>> virStateCleanup(),
>> for some important resources like(static virNetlinkEventSrvPrivatePtr 
>> server)
>> are freed unexpected. However virStateCleanup() need to use this
>> variable(server).
>>
>> The call trace of virNetlinkEventServiceStopAll:
>>
>> virNetlinkEventServiceStopAll()
>>     --> virNetlinkEventServiceStop()
>>       --> server[protocol] = NULL;   // set server to null
>>
>> The call trace of virStateCleanup():
>> virStateCleanup()
>>     -->qemuStateCleanup()
>>       -->qemuProcessStop()
>
>
> Where does qemuStateCleanup() call qemuProcessStop()?
>
>
> (If you're basing this on the comment at the top of qemuStateCleanup 
> that says "it will stop all active domains and networks", note that 
> that comment was added to the code in commit b4c282a79 on June 29, 
> 2007. That was before my time, so I don't know if it was true at that 
> time that shutting down libvirtd would stop all active domains and 
> networks, but it definitely is not true now (and hasn't been for at 
> least 10 years).
>
>
> What were the exact steps to cause this crash?
>
>
>> -->virNetDevMacVLanDeleteWithVPortProfile()
>>          -->virNetlinkEventRemoveClient()
>>            --> srv = server[protocol]
>>
>> In virNetlinkEventRemoveClient() the variable server is used again, 
>> but now it
>> is null that is freed by virNetlinkEventServiceStopAll().So it would 
>> case a crash .
>>
>> The call trace of crash:
>>
>> (gdb) bt
>> 0  __GI___pthread_mutex_lock (mutex=0x0) at 
>> /usr/src/debug/glibc/2.24-r0/git/nptl/pthread_mutex_lock.c:67
>> 1  0x00007fb0d555d0f9 in virNetlinkEventRemoveClient () from 
>> /usr/lib64/libvirt.so.0
>> 2  0x00007fb0d55551df in virNetDevMacVLanDeleteWithVPortProfile () 
>> from /usr/lib64/libvirt.so.0
>> 3  0x00007fb0c1131251 in qemuProcessStop () from 
>> /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so
>> 4  0x00007fb0c11995ea in ?? () from 
>> /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so
>> 5  0x00007fb0d5588c5b in ?? () from /usr/lib64/libvirt.so.0
>> 6  0x00007fb0d5587fe8 in ?? () from /usr/lib64/libvirt.so.0
>> 7  0x00007fb0d19533f4 in start_thread (arg=0x7fb0be17b700) at 
>> /usr/src/debug/glibc/2.24-r0/git/nptl/pthread_create.c:456
>> 8  0x00007fb0d128f10f in clone () at 
>> ../sysdeps/unix/sysv/linux/x86_64/clone.S:105
>>
>> Signed-off-by: Liu Haitao <haitao.liu at windriver.com>
>> ---
>>   src/remote/remote_daemon.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
>> index c3782971f1..7da20a6644 100644
>> --- a/src/remote/remote_daemon.c
>> +++ b/src/remote/remote_daemon.c
>> @@ -1464,8 +1464,6 @@ int main(int argc, char **argv) {
>>       /* Keep cleanup order in inverse order of startup */
>>       virNetDaemonClose(dmn);
>>   -    virNetlinkEventServiceStopAll();
>> -
>>       if (driversInitialized) {
>>           /* NB: Possible issue with timing window between 
>> driversInitialized
>>            * setting if virNetlinkEventServerStart fails */
>> @@ -1473,6 +1471,8 @@ int main(int argc, char **argv) {
>>           virStateCleanup();
>>       }
>>   +    virNetlinkEventServiceStopAll();
>> +
>>       virObjectUnref(adminProgram);
>>       virObjectUnref(srvAdm);
>>       virObjectUnref(qemuProgram);
>
>
>