[libvirt] Fix memleaks in libvirtd's message processing

[Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>]

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.

Matthias

diff --git a/daemon/dispatch.c b/daemon/dispatch.c
index a60f2f4..ddb3215 100644
--- a/daemon/dispatch.c
+++ b/daemon/dispatch.c
@@ -191,6 +191,7 @@ remoteSerializeError(struct qemud_client *client,
 
 xdr_error:
     xdr_destroy(&xdr);
+    VIR_FREE(msg);
 fatal_error:
     xdr_free((xdrproc_t)xdr_remote_error,  (char *)rerr);
     return -1;
@@ -336,6 +337,7 @@ remoteDispatchClientRequest (struct qemud_server *server,
                              struct qemud_client *client,
                              struct qemud_client_message *msg)
 {
+    int ret;
     remote_error rerr;
 
     memset(&rerr, 0, sizeof rerr);
@@ -364,7 +366,12 @@ remoteDispatchClientRequest (struct qemud_server *server,
     }
 
 error:
-    return remoteSerializeReplyError(client, &rerr, &msg->hdr);
+    ret = remoteSerializeReplyError(client, &rerr, &msg->hdr);
+
+    if (ret >= 0)
+        VIR_FREE(msg);
+
+    return ret;
 }
 
 
@@ -521,8 +528,12 @@ remoteDispatchClientCall (struct qemud_server *server,
 rpc_error:
     /* Semi-bad stuff happened, we can still try to send back
      * an RPC error message to client */
-    return remoteSerializeReplyError(client, &rerr, &msg->hdr);
+    rv = remoteSerializeReplyError(client, &rerr, &msg->hdr);
+
+    if (rv >= 0)
+        VIR_FREE(msg);
 
+    return rv;
 
 xdr_error:
     /* Seriously bad stuff happened, so we'll kill off this client

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list