[libvirt] Fix memleaks in libvirtd's message processing

[Matthias Bolte]

2009/9/29 Daniel P. Berrange <berrange at redhat.com>:
> On Sat, Sep 26, 2009 at 02:24:33AM +0200, Matthias Bolte wrote:
>> I tracked down a memleak in libvirtd's message processing. The leak
>> was introduced in commit 47cab734995fa9521b1df05d37e9978eedd8d3a2
>> "Split out code for handling incoming method call messages". This
>> commit changed the way qemud_client_message objects were reused.
>>
>> Before this commit:
>>
>> - qemudWorker() removes a message from dispatch queue and passes it to
>> remoteDispatchClientRequest()
>> - remoteDispatchClientRequest() handles the message and reuses the
>> same message for the response. If an error occurs the same message is
>> used to report it (rpc_error label). If another error occurs while
>> trying to report the first error remoteDispatchClientRequest() would
>> return -1 and the message will be freed in qemudWorker()
>>
>> After this commit:
>>
>> - qemudWorker() removes a message from dispatch queue and passes it to
>> remoteDispatchClientRequest()
>> - remoteDispatchClientRequest() calls remoteDispatchClientCall() if
>> the message it is an remote call or otherwise respond with a new error
>> message by calling remoteSerializeReplyError() and the original
>> message leaks
>> - remoteDispatchClientCall() handles the message and reuses the same
>> message for the response. If an error occurs it calls
>> remoteSerializeReplyError() and the original message leaks. If a fatal
>> error occurs remoteDispatchClientCall() returns -1 and the original
>> message will be freed in qemudWorker()
>>
>> To fix this leak the original message has to be freed if
>> remoteSerializeReplyError() succeeds. If remoteSerializeReplyError()
>> fails the original message will be freed in qemudWorker().
>>
>> In addition I came across another leak in remoteSerializeError(). If
>> an error occurs while serializing the initial error the message leaks.
>> To fix this leak the message has to be freed in case of an XDR error.
>
> ACK to your patch - this is all my fault caught up in the many times
> I re-wrote the streams support. I really hate the contract of the
> remoteDispatchClientRequest() method, it should always be responsible
> for free'ing the 'msg' regardless of return status. I had actually
> refactored things todo that, but then I rewrote it again and forget
> that I had changed the error reporting functions thus leading to
> this leak.
>
> Daniel

Okay, I've committed the patch.

Matthias