1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 00/14] Fix race on srv->nclients_unauth and some other changes
  4. View patch

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

Marc Hartmayer posted 14 patches 7 years, 5 months ago
There is a newer version of this series
[libvirt] [PATCH 02/14] rpc: Fix memory leaks @virnetserverclient
Posted by Marc Hartmayer 7 years, 5 months ago 
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@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@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);
 }
 
 
-- 
2.13.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/14] rpc: Fix memory leaks @virnetserverclient
Posted by John Ferlan 7 years, 5 months ago 

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@linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@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@redhat.com>

John

>  }
>  
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/14] rpc: Fix memory leaks @virnetserverclient
Posted by Marc Hartmayer 7 years, 4 months ago 
On Fri, Dec 15, 2017 at 01:14 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
> 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@linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@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.

Yes. Will add an patch for this. Should I remove this patch then?

>
> 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@redhat.com>
>
> John
>
>>  }
>>
>>
>>
>
--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/14] rpc: Fix memory leaks @virnetserverclient
Posted by John Ferlan 7 years, 4 months ago 

On 12/21/2017 06:32 AM, Marc Hartmayer wrote:
> On Fri, Dec 15, 2017 at 01:14 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
>> 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@linux.vnet.ibm.com>
>>> Reviewed-by: Boris Fiuczynski <fiuczy@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.
> 
> Yes. Will add an patch for this. Should I remove this patch then?
> 

Yes probably should have been explicit - the R-B below was conditional
on altering this patch to call virNetServerClientClose in the test
program instead of in virNetServerClientDispose.

John
>>
>> 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@redhat.com>
>>
>> John
>>
>>>  }
>>>
>>>
>>>
>>
> --
> Beste Grüße / Kind regards
>    Marc Hartmayer
> 
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Martina Koederitz
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
> 

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.