On 9/27/22 17:38, Jiang Jiacheng wrote: > From: jiangjiacheng <jiangjiacheng@xxxxxxxxxx> > > 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. > > Signed-off-by: jiangjiacheng <jiangjiacheng@xxxxxxxxxx> > --- > src/rpc/virnetserverclient.c | 2 ++ > src/rpc/virnetserverprogram.c | 12 +++++++++--- > 2 files changed, 11 insertions(+), 3 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); Yeah, this one is a genuine memleak. IIUC it can be reproduced by: client = virNetServerClientNew(...); virObjectUnref(client); > > diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c > index 3ddf9f0428..a813e821a3 100644 > --- a/src/rpc/virnetserverprogram.c > +++ b/src/rpc/virnetserverprogram.c > @@ -409,11 +409,15 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog, > if (virNetMessageDecodePayload(msg, dispatcher->arg_filter, arg) < 0) > goto error; > > - if (!(identity = virNetServerClientGetIdentity(client))) > + if (!(identity = virNetServerClientGetIdentity(client))) { > + xdr_free(dispatcher->arg_filter, arg); But here I believe we also need to free dispatcher->ret_filter: xdr_free(dispatcher->ret_filter, ret); and in the rest of the hunks too. But at this point, I wonder whether we shouldn't have just two places where this free is done: one in successful path and one under error label. Michal