* [PATCH] io/channel-tls: plug memory leakage on GSource
@ 2023-03-06 23:15 Matheus Tavares Bernardino
2023-03-07 9:15 ` Antoine Damhet
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Matheus Tavares Bernardino @ 2023-03-06 23:15 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, antoine.damhet, charles.frey, berrange
This leakage can be seen through test-io-channel-tls:
$ ../configure --target-list=aarch64-softmmu --enable-sanitizers
$ make ./tests/unit/test-io-channel-tls
$ ./tests/unit/test-io-channel-tls
Indirect leak of 104 byte(s) in 1 object(s) allocated from:
#0 0x7f81d1725808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
#1 0x7f81d135ae98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98)
#2 0x55616c5d4c1b in object_new_with_propv ../qom/object.c:795
#3 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
#4 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
#5 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
#6 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7f81d1725a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
#1 0x7f81d1472a20 in gnutls_dh_params_init (/lib/x86_64-linux-gnu/libgnutls.so.30+0x46a20)
#2 0x55616c6485ff in qcrypto_tls_creds_x509_load ../crypto/tlscredsx509.c:634
#3 0x55616c648ba2 in qcrypto_tls_creds_x509_complete ../crypto/tlscredsx509.c:694
#4 0x55616c5e1fea in user_creatable_complete ../qom/object_interfaces.c:28
#5 0x55616c5d4c8c in object_new_with_propv ../qom/object.c:807
#6 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
#7 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
#8 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
#9 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
...
SUMMARY: AddressSanitizer: 49143 byte(s) leaked in 184 allocation(s).
The docs for `g_source_add_child_source(source, child_source)` says
"source will hold a reference on child_source while child_source is
attached to it." Therefore, we should unreference the child source at
`qio_channel_tls_read_watch()` after attaching it to `source`. With this
change, ./tests/unit/test-io-channel-tls shows no leakages.
Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
io/channel-tls.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 8052945ba0..5a7a3d48d6 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -446,6 +446,7 @@ qio_channel_tls_read_watch(QIOChannelTLS *tioc, GSource *source)
object_ref(OBJECT(tioc));
g_source_add_child_source(source, child);
+ g_source_unref(child);
}
static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
--
2.37.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] io/channel-tls: plug memory leakage on GSource
2023-03-06 23:15 [PATCH] io/channel-tls: plug memory leakage on GSource Matheus Tavares Bernardino
@ 2023-03-07 9:15 ` Antoine Damhet
2023-03-07 10:13 ` Daniel P. Berrangé
2023-03-07 10:16 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 4+ messages in thread
From: Antoine Damhet @ 2023-03-07 9:15 UTC (permalink / raw
To: Matheus Tavares Bernardino
Cc: qemu-devel, qemu-trivial, charles.frey, berrange
[-- Attachment #1: Type: text/plain, Size: 3014 bytes --]
Nice catch !
On Mon, Mar 06, 2023 at 08:15:21PM -0300, Matheus Tavares Bernardino wrote:
> This leakage can be seen through test-io-channel-tls:
>
> $ ../configure --target-list=aarch64-softmmu --enable-sanitizers
> $ make ./tests/unit/test-io-channel-tls
> $ ./tests/unit/test-io-channel-tls
>
> Indirect leak of 104 byte(s) in 1 object(s) allocated from:
> #0 0x7f81d1725808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
> #1 0x7f81d135ae98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98)
> #2 0x55616c5d4c1b in object_new_with_propv ../qom/object.c:795
> #3 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
> #4 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
> #5 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
> #6 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
>
> Indirect leak of 32 byte(s) in 1 object(s) allocated from:
> #0 0x7f81d1725a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
> #1 0x7f81d1472a20 in gnutls_dh_params_init (/lib/x86_64-linux-gnu/libgnutls.so.30+0x46a20)
> #2 0x55616c6485ff in qcrypto_tls_creds_x509_load ../crypto/tlscredsx509.c:634
> #3 0x55616c648ba2 in qcrypto_tls_creds_x509_complete ../crypto/tlscredsx509.c:694
> #4 0x55616c5e1fea in user_creatable_complete ../qom/object_interfaces.c:28
> #5 0x55616c5d4c8c in object_new_with_propv ../qom/object.c:807
> #6 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
> #7 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
> #8 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
> #9 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
>
> ...
>
> SUMMARY: AddressSanitizer: 49143 byte(s) leaked in 184 allocation(s).
>
> The docs for `g_source_add_child_source(source, child_source)` says
> "source will hold a reference on child_source while child_source is
> attached to it." Therefore, we should unreference the child source at
> `qio_channel_tls_read_watch()` after attaching it to `source`. With this
> change, ./tests/unit/test-io-channel-tls shows no leakages.
>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Reviewed-by: Antoine Damhet <antoine.damhet@shadow.tech>
> ---
> io/channel-tls.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/io/channel-tls.c b/io/channel-tls.c
> index 8052945ba0..5a7a3d48d6 100644
> --- a/io/channel-tls.c
> +++ b/io/channel-tls.c
> @@ -446,6 +446,7 @@ qio_channel_tls_read_watch(QIOChannelTLS *tioc, GSource *source)
> object_ref(OBJECT(tioc));
>
> g_source_add_child_source(source, child);
> + g_source_unref(child);
> }
>
> static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
> --
> 2.37.2
>
--
Antoine 'xdbob' Damhet
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] io/channel-tls: plug memory leakage on GSource
2023-03-06 23:15 [PATCH] io/channel-tls: plug memory leakage on GSource Matheus Tavares Bernardino
2023-03-07 9:15 ` Antoine Damhet
@ 2023-03-07 10:13 ` Daniel P. Berrangé
2023-03-07 10:16 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2023-03-07 10:13 UTC (permalink / raw
To: Matheus Tavares Bernardino
Cc: qemu-devel, qemu-trivial, antoine.damhet, charles.frey
On Mon, Mar 06, 2023 at 08:15:21PM -0300, Matheus Tavares Bernardino wrote:
> This leakage can be seen through test-io-channel-tls:
>
> $ ../configure --target-list=aarch64-softmmu --enable-sanitizers
> $ make ./tests/unit/test-io-channel-tls
> $ ./tests/unit/test-io-channel-tls
>
> Indirect leak of 104 byte(s) in 1 object(s) allocated from:
> #0 0x7f81d1725808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
> #1 0x7f81d135ae98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98)
> #2 0x55616c5d4c1b in object_new_with_propv ../qom/object.c:795
> #3 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
> #4 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
> #5 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
> #6 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
>
> Indirect leak of 32 byte(s) in 1 object(s) allocated from:
> #0 0x7f81d1725a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
> #1 0x7f81d1472a20 in gnutls_dh_params_init (/lib/x86_64-linux-gnu/libgnutls.so.30+0x46a20)
> #2 0x55616c6485ff in qcrypto_tls_creds_x509_load ../crypto/tlscredsx509.c:634
> #3 0x55616c648ba2 in qcrypto_tls_creds_x509_complete ../crypto/tlscredsx509.c:694
> #4 0x55616c5e1fea in user_creatable_complete ../qom/object_interfaces.c:28
> #5 0x55616c5d4c8c in object_new_with_propv ../qom/object.c:807
> #6 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
> #7 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
> #8 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
> #9 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
>
> ...
>
> SUMMARY: AddressSanitizer: 49143 byte(s) leaked in 184 allocation(s).
>
> The docs for `g_source_add_child_source(source, child_source)` says
> "source will hold a reference on child_source while child_source is
> attached to it." Therefore, we should unreference the child source at
> `qio_channel_tls_read_watch()` after attaching it to `source`. With this
> change, ./tests/unit/test-io-channel-tls shows no leakages.
Yes, the other places in QEMU that call g_source_add_child_source also
unref the source after adding it.
>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
> io/channel-tls.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
and queued.
>
> diff --git a/io/channel-tls.c b/io/channel-tls.c
> index 8052945ba0..5a7a3d48d6 100644
> --- a/io/channel-tls.c
> +++ b/io/channel-tls.c
> @@ -446,6 +446,7 @@ qio_channel_tls_read_watch(QIOChannelTLS *tioc, GSource *source)
> object_ref(OBJECT(tioc));
>
> g_source_add_child_source(source, child);
> + g_source_unref(child);
> }
>
> static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
> --
> 2.37.2
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] io/channel-tls: plug memory leakage on GSource
2023-03-06 23:15 [PATCH] io/channel-tls: plug memory leakage on GSource Matheus Tavares Bernardino
2023-03-07 9:15 ` Antoine Damhet
2023-03-07 10:13 ` Daniel P. Berrangé
@ 2023-03-07 10:16 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-07 10:16 UTC (permalink / raw
To: Matheus Tavares Bernardino, qemu-devel
Cc: qemu-trivial, antoine.damhet, charles.frey, berrange
On 7/3/23 00:15, Matheus Tavares Bernardino wrote:
> This leakage can be seen through test-io-channel-tls:
>
> $ ../configure --target-list=aarch64-softmmu --enable-sanitizers
> $ make ./tests/unit/test-io-channel-tls
> $ ./tests/unit/test-io-channel-tls
>
> Indirect leak of 104 byte(s) in 1 object(s) allocated from:
> #0 0x7f81d1725808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
> #1 0x7f81d135ae98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98)
> #2 0x55616c5d4c1b in object_new_with_propv ../qom/object.c:795
> #3 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
> #4 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
> #5 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
> #6 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
>
> Indirect leak of 32 byte(s) in 1 object(s) allocated from:
> #0 0x7f81d1725a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
> #1 0x7f81d1472a20 in gnutls_dh_params_init (/lib/x86_64-linux-gnu/libgnutls.so.30+0x46a20)
> #2 0x55616c6485ff in qcrypto_tls_creds_x509_load ../crypto/tlscredsx509.c:634
> #3 0x55616c648ba2 in qcrypto_tls_creds_x509_complete ../crypto/tlscredsx509.c:694
> #4 0x55616c5e1fea in user_creatable_complete ../qom/object_interfaces.c:28
> #5 0x55616c5d4c8c in object_new_with_propv ../qom/object.c:807
> #6 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768
> #7 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70
> #8 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158
> #9 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
>
> ...
>
> SUMMARY: AddressSanitizer: 49143 byte(s) leaked in 184 allocation(s).
>
> The docs for `g_source_add_child_source(source, child_source)` says
> "source will hold a reference on child_source while child_source is
> attached to it." Therefore, we should unreference the child source at
> `qio_channel_tls_read_watch()` after attaching it to `source`. With this
> change, ./tests/unit/test-io-channel-tls shows no leakages.
>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
> io/channel-tls.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/io/channel-tls.c b/io/channel-tls.c
> index 8052945ba0..5a7a3d48d6 100644
> --- a/io/channel-tls.c
> +++ b/io/channel-tls.c
> @@ -446,6 +446,7 @@ qio_channel_tls_read_watch(QIOChannelTLS *tioc, GSource *source)
> object_ref(OBJECT(tioc));
>
> g_source_add_child_source(source, child);
> + g_source_unref(child);
Or declare child with 'g_autoptr(GSource)'. The difference is
only a matter of style, so I'll let Daniel to decide what is
preferred. Regardless:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-07 10:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-06 23:15 [PATCH] io/channel-tls: plug memory leakage on GSource Matheus Tavares Bernardino
2023-03-07 9:15 ` Antoine Damhet
2023-03-07 10:13 ` Daniel P. Berrangé
2023-03-07 10:16 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.