[libvirt] Fix memleaks in libvirtd's message processing

[Daniel P. Berrange]

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
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|