Re: [PATCH v3] rpc: fix memory leak in virNetServerClientNew and virNetServerProgramDispatchCall

From: jiangjiacheng <jiangjiacheng(a)huawei.com&gt; In virNetServerProgramDispatchCall, The arg is passed as a void* and used to point to a certain struct depended on the dispatcher, so I think it's the memory of the struct's member that leaks and this memory shuld be freed by xdr_free. In virNetServerClientNew, client->rx is assigned by invoking virNetServerClientNew, but isn't freed if client->privateData's initialization failed, which leads to a memory leak. Thanks to Liang Peng's suggestion, put virNetMessageFree(client->rx) into virNetServerClientDispose() to release the memory. diff to v2: - virNetServerProgramDispatchCall, only free the memory in successful path and error label.

Signed-off-by: jiangjiacheng <jiangjiacheng(a)huawei.com&gt; ---

src/rpc/virnetserverclient.c | 2 ++ src/rpc/virnetserverprogram.c | 22 ++++++++++------------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index a7d2dfa795..30f6af7be5 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -931,6 +931,8 @@ void virNetServerClientDispose(void *obj) PROBE(RPC_SERVER_CLIENT_DISPOSE, "client=%p", client); + if (client->rx) + virNetMessageFree(client->rx); if (client->privateData) client->privateDataFreeFunc(client->privateData); diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 3ddf9f0428..94660f867a 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -368,7 +368,7 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog, g_autofree char *arg = NULL; g_autofree char *ret = NULL; int rv = -1; - virNetServerProgramProc *dispatcher; + virNetServerProgramProc *dispatcher = NULL; virNetMessageError rerr; size_t i; g_autoptr(virIdentity) identity = NULL; @@ -446,8 +446,6 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog, msg->nfds = 0; } - xdr_free(dispatcher->arg_filter, arg); - if (rv < 0) goto error; @@ -460,28 +458,28 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog, /*msg->header.serial = msg->header.serial;*/ msg->header.status = VIR_NET_OK; - if (virNetMessageEncodeHeader(msg) < 0) { - xdr_free(dispatcher->ret_filter, ret); + if (virNetMessageEncodeHeader(msg) < 0) goto error; - } if (msg->nfds && - virNetMessageEncodeNumFDs(msg) < 0) { - xdr_free(dispatcher->ret_filter, ret); + virNetMessageEncodeNumFDs(msg) < 0) goto error; - } - if (virNetMessageEncodePayload(msg, dispatcher->ret_filter, ret) < 0) { - xdr_free(dispatcher->ret_filter, ret); + if (virNetMessageEncodePayload(msg, dispatcher->ret_filter, ret) < 0) goto error; - } + xdr_free(dispatcher->arg_filter, arg); xdr_free(dispatcher->ret_filter, ret); /* Put reply on end of tx queue to send out */ return virNetServerClientSendMessage(client, msg); error: + if (dispatcher) { + xdr_free(dispatcher->arg_filter, arg); + xdr_free(dispatcher->ret_filter, ret); + } +