tests/qtest/migration/tls-tests.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
If you run the TLS tests under a clang undefined-behaviour sanitizer build
it will fall over like this:
../../tests/unit/crypto-tls-psk-helpers.c:53: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
#0 0x62bd810762ee in test_tls_psk_cleanup /home/pm215/qemu/build/clang/../../tests/unit/crypto-tls-psk-helpers.c:53:5
#1 0x62bd81073f89 in migrate_hook_end_tls_psk /home/pm215/qemu/build/clang/../../tests/qtest/migration/tls-tests.c:101:5
#2 0x62bd81062ef0 in test_precopy_common /home/pm215/qemu/build/clang/../../tests/qtest/migration/framework.c:947:9
This happens because test_precopy_tcp_no_tls() uses a custom start_hook
that only sets a couple of parameters, but reuses the tsk_psk end_hook.
However, the end_hook runs cleanup that assumes that the data was set
up by migrate_hook_start_tls_psk_common(). In particular, it will
unconditionally call test_tls_psk_cleanup(data->pskfile), and
test_tls_psk_cleanup() will unconditionally unlink() the filename it
is passed, which is undefined behaviour if you pass it a NULL pointer.
Instead of creating a TestMigrateTLSPSKData struct which we never set
any fields in and requiring the migrate_hook_end_tls_psk() hook to
cope with that, don't allocate the struct in the start_hook. Then
there is nothing we need to clean up, and we can set the end_hook
to NULL (which the test framework will interpret as "don't call
any end_hook").
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Not sure when this would have appeared, so cc'ing stable as it
seems safe enough to backport just in case.
---
tests/qtest/migration/tls-tests.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
index bf0bb06a29..4ce7f6c676 100644
--- a/tests/qtest/migration/tls-tests.c
+++ b/tests/qtest/migration/tls-tests.c
@@ -488,20 +488,18 @@ static void test_precopy_tcp_tls_psk_mismatch(char *name, MigrateCommon *args)
static void *migrate_hook_start_no_tls(QTestState *from, QTestState *to)
{
- struct TestMigrateTLSPSKData *data =
- g_new0(struct TestMigrateTLSPSKData, 1);
-
migrate_set_parameter_null(from, "tls-creds");
migrate_set_parameter_null(to, "tls-creds");
- return data;
+ return NULL;
}
static void test_precopy_tcp_no_tls(char *name, MigrateCommon *args)
{
args->listen_uri = "tcp:127.0.0.1:0";
args->start_hook = migrate_hook_start_no_tls;
- args->end_hook = migrate_hook_end_tls_psk;
+ /* the no_tls start hook requires no cleanup actions */
+ args->end_hook = NULL;
test_precopy_common(args);
}
--
2.43.0
On Thu, Feb 12, 2026 at 11:47:47AM +0000, Peter Maydell wrote: > If you run the TLS tests under a clang undefined-behaviour sanitizer build > it will fall over like this: > > ../../tests/unit/crypto-tls-psk-helpers.c:53: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 > #0 0x62bd810762ee in test_tls_psk_cleanup /home/pm215/qemu/build/clang/../../tests/unit/crypto-tls-psk-helpers.c:53:5 > #1 0x62bd81073f89 in migrate_hook_end_tls_psk /home/pm215/qemu/build/clang/../../tests/qtest/migration/tls-tests.c:101:5 > #2 0x62bd81062ef0 in test_precopy_common /home/pm215/qemu/build/clang/../../tests/qtest/migration/framework.c:947:9 > > This happens because test_precopy_tcp_no_tls() uses a custom start_hook > that only sets a couple of parameters, but reuses the tsk_psk end_hook. > However, the end_hook runs cleanup that assumes that the data was set > up by migrate_hook_start_tls_psk_common(). In particular, it will > unconditionally call test_tls_psk_cleanup(data->pskfile), and > test_tls_psk_cleanup() will unconditionally unlink() the filename it > is passed, which is undefined behaviour if you pass it a NULL pointer. > > Instead of creating a TestMigrateTLSPSKData struct which we never set > any fields in and requiring the migrate_hook_end_tls_psk() hook to > cope with that, don't allocate the struct in the start_hook. Then > there is nothing we need to clean up, and we can set the end_hook > to NULL (which the test framework will interpret as "don't call > any end_hook"). > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com> -- Peter Xu
Peter Maydell <peter.maydell@linaro.org> writes: > If you run the TLS tests under a clang undefined-behaviour sanitizer build > it will fall over like this: > > ../../tests/unit/crypto-tls-psk-helpers.c:53: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 > #0 0x62bd810762ee in test_tls_psk_cleanup /home/pm215/qemu/build/clang/../../tests/unit/crypto-tls-psk-helpers.c:53:5 > #1 0x62bd81073f89 in migrate_hook_end_tls_psk /home/pm215/qemu/build/clang/../../tests/qtest/migration/tls-tests.c:101:5 > #2 0x62bd81062ef0 in test_precopy_common /home/pm215/qemu/build/clang/../../tests/qtest/migration/framework.c:947:9 > > This happens because test_precopy_tcp_no_tls() uses a custom start_hook > that only sets a couple of parameters, but reuses the tsk_psk end_hook. > However, the end_hook runs cleanup that assumes that the data was set > up by migrate_hook_start_tls_psk_common(). In particular, it will > unconditionally call test_tls_psk_cleanup(data->pskfile), and > test_tls_psk_cleanup() will unconditionally unlink() the filename it > is passed, which is undefined behaviour if you pass it a NULL pointer. > > Instead of creating a TestMigrateTLSPSKData struct which we never set > any fields in and requiring the migrate_hook_end_tls_psk() hook to > cope with that, don't allocate the struct in the start_hook. Then > there is nothing we need to clean up, and we can set the end_hook > to NULL (which the test framework will interpret as "don't call > any end_hook"). > Thanks! My pull-request script should have caught this. I recently excluded the TLS tests because they break ASAN in weird ways. Reviewed-by: Fabiano Rosas <farosas@suse.de> > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Not sure when this would have appeared, so cc'ing stable as it > seems safe enough to backport just in case. It's from this devel cycle.
© 2016 - 2026 Red Hat, Inc.