From: jiangjiacheng <jiangjiacheng@huawei.com>

In virNetServerClientNew, client->rx is be assigned by invoking virNetServerClientNew,
but isn't freed if client->privateData's initialization failed, which leads to a
memory leak. And in virNetServerProgramDispatchCall, the error path doesn't free the
memory of dispatcher->arg_filter, which leads to a memory leak.

Signed-off-by: jiangjiacheng <jiangjiacheng@huawei.com>
---
 src/rpc/virnetserverclient.c  |  1 +
 src/rpc/virnetserverprogram.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index a7d2dfa795..75df16fe96 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -443,6 +443,7 @@ virNetServerClient *virNetServerClientNew(unsigned long long id,
         return NULL;
 
     if (!(client->privateData = privNew(client, privOpaque))) {
+        virNetMessageFree(client->rx);
         virObjectUnref(client);
         return NULL;
     }
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);
         goto error;
+    }
 
-    if (virIdentitySetCurrent(identity) < 0)
+    if (virIdentitySetCurrent(identity) < 0) {
+        xdr_free(dispatcher->arg_filter, arg);
         goto error;
+    }
 
     /*
      * When the RPC handler is called:
@@ -427,8 +431,10 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog,
      */
     rv = (dispatcher->func)(server, client, msg, &rerr, arg, ret);
 
-    if (virIdentitySetCurrent(NULL) < 0)
+    if (virIdentitySetCurrent(NULL) < 0) {
+        xdr_free(dispatcher->arg_filter, arg);
         goto error;
+    }
 
     /*
      * If rv == 1, this indicates the dispatch func has
-- 
2.33.0

On 09/09/2022 14:10, Jiacheng Jiang wrote:
> From: jiangjiacheng <jiangjiacheng@huawei.com>
> 
> In virNetServerClientNew, client->rx is be assigned by invoking virNetServerClientNew,
> but isn't freed if client->privateData's initialization failed, which leads to a
> memory leak. And in virNetServerProgramDispatchCall, the error path doesn't free the
> memory of dispatcher->arg_filter, which leads to a memory leak.

I think it's memory `arg` points to instead of dispatcher->arg_filter 
that leaks?

> 
> Signed-off-by: jiangjiacheng <jiangjiacheng@huawei.com>
> ---
>   src/rpc/virnetserverclient.c  |  1 +
>   src/rpc/virnetserverprogram.c | 12 +++++++++---
>   2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index a7d2dfa795..75df16fe96 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -443,6 +443,7 @@ virNetServerClient *virNetServerClientNew(unsigned long long id,
>           return NULL;
>   
>       if (!(client->privateData = privNew(client, privOpaque))) {
> +        virNetMessageFree(client->rx);

Maybe it's better to put this line into `virNetServerClientDispose`?

>           virObjectUnref(client);
>           return NULL;
>       }
> 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);
>           goto error;
> +    }
>   
> -    if (virIdentitySetCurrent(identity) < 0)
> +    if (virIdentitySetCurrent(identity) < 0) {
> +        xdr_free(dispatcher->arg_filter, arg);
>           goto error;
> +    }
>   
>       /*
>        * When the RPC handler is called:
> @@ -427,8 +431,10 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog,
>        */
>       rv = (dispatcher->func)(server, client, msg, &rerr, arg, ret);
>   
> -    if (virIdentitySetCurrent(NULL) < 0)
> +    if (virIdentitySetCurrent(NULL) < 0) {
> +        xdr_free(dispatcher->arg_filter, arg);
>           goto error;
> +    }
>   
>       /*
>        * If rv == 1, this indicates the dispatch func has

Thanks,
Peng