All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>, qemu-devel@nongnu.org
Subject: [PULL 1/3] tests/qtest/migration-test: Fix unlink error and memory leaks
Date: Sun,  4 Dec 2022 08:30:41 +0100	[thread overview]
Message-ID: <20221204073043.54402-2-thuth@redhat.com> (raw)
In-Reply-To: <20221204073043.54402-1-thuth@redhat.com>

When running the migration test compiled with Clang from Fedora 37
and sanitizers enabled, there is an error complaining about unlink():

 ../tests/qtest/migration-test.c:1072:12: runtime error: null pointer
  passed as argument 1, which is declared to never be null
 /usr/include/unistd.h:858:48: note: nonnull attribute specified here
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
  ../tests/qtest/migration-test.c:1072:12 in
 (test program exited with status code 1)
 TAP parsing error: Too few tests run (expected 33, got 20)

The data->clientcert and data->clientkey pointers can indeed be unset
in some tests, so we have to check them before calling unlink() with
those.

While we're at it, I also noticed that the code is only freeing
some but not all of the allocated strings in this function, and
indeed, valgrind is also complaining about memory leaks here.
So let's call g_free() on all allocated strings to avoid leaking
memory here.

Message-Id: <20221125083054.117504-1-thuth@redhat.com>
Tested-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/migration-test.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 442998d9eb..dbde726adf 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1066,15 +1066,27 @@ test_migrate_tls_x509_finish(QTestState *from,
     TestMigrateTLSX509Data *data = opaque;
 
     test_tls_cleanup(data->keyfile);
+    g_free(data->keyfile);
+
     unlink(data->cacert);
+    g_free(data->cacert);
     unlink(data->servercert);
+    g_free(data->servercert);
     unlink(data->serverkey);
-    unlink(data->clientcert);
-    unlink(data->clientkey);
-    rmdir(data->workdir);
+    g_free(data->serverkey);
 
+    if (data->clientcert) {
+        unlink(data->clientcert);
+        g_free(data->clientcert);
+    }
+    if (data->clientkey) {
+        unlink(data->clientkey);
+        g_free(data->clientkey);
+    }
+
+    rmdir(data->workdir);
     g_free(data->workdir);
-    g_free(data->keyfile);
+
     g_free(data);
 }
 #endif /* CONFIG_TASN1 */
-- 
2.31.1



  reply	other threads:[~2022-12-04  7:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-04  7:30 [PULL 0/3] Optional fixes for inclusion into QEMU 7.2.0-rc4 Thomas Huth
2022-12-04  7:30 ` Thomas Huth [this message]
2022-12-04  7:30 ` [PULL 2/3] target/s390x/tcg: Fix and improve the SACF instruction Thomas Huth
2022-12-04  7:30 ` [PULL 3/3] hw/display/next-fb: Fix comment typo Thomas Huth
2022-12-04 23:47 ` [PULL 0/3] Optional fixes for inclusion into QEMU 7.2.0-rc4 Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221204073043.54402-2-thuth@redhat.com \
    --to=thuth@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.