[libvirt] [PATCH 02/14] rpc: Fix memory leaks @virnetserverclient

[John Ferlan]

On 12/12/2017 06:36 AM, Marc Hartmayer wrote:
> Direct leak of 104 byte(s) in 1 object(s) allocated from:
>     #0 0x7f904bfbe12b  (/lib64/liblsan.so.0+0xe12b)
>     #1 0x7f904ba0ad67 in virAlloc ../../src/util/viralloc.c:144
>     #2 0x7f904bbc11a4 in virNetMessageNew ../../src/rpc/virnetmessage.c:42
>     #3 0x7f904bbb8e77 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:392
>     #4 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440
>     #5 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55
>     #6 0x403bed in virTestRun ../../tests/testutils.c:180
>     #7 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146
>     #8 0x404c80 in virTestMain ../../tests/testutils.c:1119
>     #9 0x4030d5 in main ../../tests/virnetserverclienttest.c:152
>     #10 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889)
> 
> Indirect leak of 4 byte(s) in 1 object(s) allocated from:
>     #0 0x7f904bfbe12b  (/lib64/liblsan.so.0+0xe12b)
>     #1 0x7f904ba0adc7 in virAllocN ../../src/util/viralloc.c:191
>     #2 0x7f904bbb8ec7 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:395
>     #3 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440
>     #4 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55
>     #5 0x403bed in virTestRun ../../tests/testutils.c:180
>     #6 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146
>     #7 0x404c80 in virTestMain ../../tests/testutils.c:1119
>     #8 0x4030d5 in main ../../tests/virnetserverclienttest.c:152
>     #9 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889)
> 
> SUMMARY: LeakSanitizer: 108 byte(s) leaked in 2 allocation(s).
> 
> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> ---
>  src/rpc/virnetserverclient.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index 6e086b7b4e2b..b454a3ff6992 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -948,6 +948,9 @@ void virNetServerClientDispose(void *obj)
>      virObjectUnref(client->tlsCtxt);
>  #endif
>      virObjectUnref(client->sock);
> +
> +    virNetMessageFree(client->rx);
> +    virNetMessageFree(client->tx);

Makes me curious why virNetServerClientClose isn't doing the Free for
the rx/tx.  Oh, I see, the test program isn't calling it.

I think this is the right idea, but the issue is in the test program
doing the Unref before calling virNetServerClientClose.

If you see how other tests/code handle this, the client usually gets
added to the net server [n]clients list (virNetServerAddClient) and
later when netserver is disposed the nclients is perused and calls
virNetServerClientClose prior to the Unref as well for each client.

So if you make the right call in tests/virnetserverclienttest.c, then
you have my

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

>  }
>  
>  
>