diff mbox series

[for-9.2,4/9] tests/qtest/migration-helpers: Fix migrate_get_socket_address() leak

Message ID 20240820144912.320744-5-peter.maydell@linaro.org
State New
Headers show
Series tests/qtest/migration-test: Fix various leaks | expand

Commit Message

Peter Maydell Aug. 20, 2024, 2:49 p.m. UTC 
In migrate_get_socket_address() we leak the SocketAddressList:
 (cd build/asan && \
  ASAN_OPTIONS="fast_unwind_on_malloc=0:strip_path_prefix=/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../"
  QTEST_QEMU_BINARY=./qemu-system-x86_64 \
  ./tests/qtest/migration-test --tap -k -p /x86_64/migration/multifd/tcp/tls/psk/match )

[...]
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x563d7f22f318 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f318) (BuildId: 2ad6282fb5d076c863ab87f41a345d46dc965ded)
    #1 0x7f9de3b39c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    #2 0x563d7f3a119c in qobject_input_start_list qapi/qobject-input-visitor.c:336:17
    #3 0x563d7f390fbf in visit_start_list qapi/qapi-visit-core.c:80:10
    #4 0x563d7f3882ef in visit_type_SocketAddressList /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qapi/qapi-visit-sockets.c:519:10
    #5 0x563d7f3658c9 in migrate_get_socket_address tests/qtest/migration-helpers.c:97:5
    #6 0x563d7f362e24 in migrate_get_connect_uri tests/qtest/migration-helpers.c:111:13
    #7 0x563d7f362bb2 in migrate_qmp tests/qtest/migration-helpers.c:222:23
    #8 0x563d7f3533cd in test_precopy_common tests/qtest/migration-test.c:1817:5
    #9 0x563d7f34dc1c in test_multifd_tcp_tls_psk_match tests/qtest/migration-test.c:3185:5
    #10 0x563d7f365337 in migration_test_wrapper tests/qtest/migration-helpers.c:458:5

The code fishes out the SocketAddress from the list to return it, and the
callers are freeing that, but nothing frees the list.

Since this function is called in only two places, the simple fix is to
make it return the SocketAddressList rather than just a SocketAddress,
and then the callers can easily access the SocketAddress, and free
the whole SocketAddressList when they're done.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/migration-helpers.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Comments

Fabiano Rosas Aug. 22, 2024, 12:24 p.m. UTC | #1 
Peter Maydell <peter.maydell@linaro.org> writes:

> In migrate_get_socket_address() we leak the SocketAddressList:
>  (cd build/asan && \
>   ASAN_OPTIONS="fast_unwind_on_malloc=0:strip_path_prefix=/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../"
>   QTEST_QEMU_BINARY=./qemu-system-x86_64 \
>   ./tests/qtest/migration-test --tap -k -p /x86_64/migration/multifd/tcp/tls/psk/match )
>
> [...]
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
>     #0 0x563d7f22f318 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f318) (BuildId: 2ad6282fb5d076c863ab87f41a345d46dc965ded)
>     #1 0x7f9de3b39c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
>     #2 0x563d7f3a119c in qobject_input_start_list qapi/qobject-input-visitor.c:336:17
>     #3 0x563d7f390fbf in visit_start_list qapi/qapi-visit-core.c:80:10
>     #4 0x563d7f3882ef in visit_type_SocketAddressList /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qapi/qapi-visit-sockets.c:519:10
>     #5 0x563d7f3658c9 in migrate_get_socket_address tests/qtest/migration-helpers.c:97:5
>     #6 0x563d7f362e24 in migrate_get_connect_uri tests/qtest/migration-helpers.c:111:13
>     #7 0x563d7f362bb2 in migrate_qmp tests/qtest/migration-helpers.c:222:23
>     #8 0x563d7f3533cd in test_precopy_common tests/qtest/migration-test.c:1817:5
>     #9 0x563d7f34dc1c in test_multifd_tcp_tls_psk_match tests/qtest/migration-test.c:3185:5
>     #10 0x563d7f365337 in migration_test_wrapper tests/qtest/migration-helpers.c:458:5
>
> The code fishes out the SocketAddress from the list to return it, and the
> callers are freeing that, but nothing frees the list.
>
> Since this function is called in only two places, the simple fix is to
> make it return the SocketAddressList rather than just a SocketAddress,
> and then the callers can easily access the SocketAddress, and free
> the whole SocketAddressList when they're done.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Fabiano Rosas <farosas@suse.de>
diff mbox series

Patch

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 84f49db85e0..7cbb9831e76 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -82,11 +82,10 @@  static QDict *SocketAddress_to_qdict(SocketAddress *addr)
     return dict;
 }
 
-static SocketAddress *migrate_get_socket_address(QTestState *who)
+static SocketAddressList *migrate_get_socket_address(QTestState *who)
 {
     QDict *rsp;
     SocketAddressList *addrs;
-    SocketAddress *addr;
     Visitor *iv = NULL;
     QObject *object;
 
@@ -95,36 +94,35 @@  static SocketAddress *migrate_get_socket_address(QTestState *who)
 
     iv = qobject_input_visitor_new(object);
     visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
-    addr = addrs->value;
     visit_free(iv);
 
     qobject_unref(rsp);
-    return addr;
+    return addrs;
 }
 
 static char *
 migrate_get_connect_uri(QTestState *who)
 {
-    SocketAddress *addrs;
+    SocketAddressList *addrs;
     char *connect_uri;
 
     addrs = migrate_get_socket_address(who);
-    connect_uri = SocketAddress_to_str(addrs);
+    connect_uri = SocketAddress_to_str(addrs->value);
 
-    qapi_free_SocketAddress(addrs);
+    qapi_free_SocketAddressList(addrs);
     return connect_uri;
 }
 
 static QDict *
 migrate_get_connect_qdict(QTestState *who)
 {
-    SocketAddress *addrs;
+    SocketAddressList *addrs;
     QDict *connect_qdict;
 
     addrs = migrate_get_socket_address(who);
-    connect_qdict = SocketAddress_to_qdict(addrs);
+    connect_qdict = SocketAddress_to_qdict(addrs->value);
 
-    qapi_free_SocketAddress(addrs);
+    qapi_free_SocketAddressList(addrs);
     return connect_qdict;
 }