From 4877c0838d8f44633bff742240776105074a3d05 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Apr 2026 11:45:26 +0200 Subject: [PATCH] Fix an infinite loop if cleanup fails in some tests Don't call a macro that does `goto exit` on failure after the `exit:` label: that would cause an infinite loop if something does go wrong. Generally, cleanup functions don't error out, so it is unlikely to be a problem in practice. If an error does happen during cleanup, it's probably due to memory corruption caused by a bug that happened earlier, and that is likely to have been detected in an earlier function. So we don't really need to assert the return code of functions called during cleanup, and normally we don't. Only a few places did so, wrongly. I found the problematic places with ``` ag 'exit:[^}]*(PSA_ASSERT|TEST_ASSERT|TEST_EQUAL)' tests/suites/*.function ``` Signed-off-by: Gilles Peskine --- tests/suites/test_suite_pk.function | 2 +- tests/suites/test_suite_psa_crypto.function | 14 +++++++------- ...est_suite_psa_crypto_driver_wrappers.function | 4 ++-- tests/suites/test_suite_psa_crypto_pake.function | 16 +++++++++------- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index a8cf711098..9bfbb37bc9 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -781,7 +781,7 @@ void pk_can_do_ext(int opaque_key, int key_type, int key_usage, int key_alg, exit: psa_reset_key_attributes(&attributes); - PSA_ASSERT(psa_destroy_key(key)); + psa_destroy_key(key); mbedtls_pk_free(&pk); USE_PSA_DONE(); } diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index fb278ddbda..6d560928b9 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -2411,7 +2411,7 @@ void aead_key_policy(int policy_usage_arg, } exit: - PSA_ASSERT(psa_aead_abort(&operation)); + psa_aead_abort(&operation); psa_destroy_key(key); PSA_DONE(); } @@ -3030,7 +3030,7 @@ void hash_compute_fail(int alg_arg, data_t *input, } exit: - PSA_ASSERT(psa_hash_abort(&operation)); + psa_hash_abort(&operation); mbedtls_free(output); PSA_DONE(); } @@ -3069,7 +3069,7 @@ void hash_compare_fail(int alg_arg, data_t *input, } exit: - PSA_ASSERT(psa_hash_abort(&operation)); + psa_hash_abort(&operation); PSA_DONE(); } /* END_CASE */ @@ -3173,7 +3173,7 @@ void hash_compute_compare(int alg_arg, data_t *input, } exit: - PSA_ASSERT(psa_hash_abort(&operation)); + psa_hash_abort(&operation); PSA_DONE(); } /* END_CASE */ @@ -4345,7 +4345,7 @@ void cipher_alg_without_iv(int alg_arg, int key_type_arg, data_t *key_data, output, output_length); exit: - PSA_ASSERT(psa_cipher_abort(&operation)); + psa_cipher_abort(&operation); mbedtls_free(output); psa_cipher_abort(&operation); psa_destroy_key(key); @@ -10967,8 +10967,8 @@ void ecjpake_setup(int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, } exit: - PSA_ASSERT(psa_destroy_key(key)); - PSA_ASSERT(psa_pake_abort(&operation)); + psa_destroy_key(key); + psa_pake_abort(&operation); mbedtls_free(output_buffer); PSA_DONE(); } diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index f5bed37c85..91b1ff60e5 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -2899,7 +2899,7 @@ void aead_encrypt_setup(int key_type_arg, data_t *key_data, exit: /* Cleanup */ - PSA_ASSERT(psa_destroy_key(key)); + psa_destroy_key(key); mbedtls_free(output_data); PSA_DONE(); mbedtls_test_driver_aead_hooks = mbedtls_test_driver_aead_hooks_init(); @@ -3001,7 +3001,7 @@ void aead_decrypt_setup(int key_type_arg, data_t *key_data, } exit: - PSA_ASSERT(psa_destroy_key(key)); + psa_destroy_key(key); mbedtls_free(output_data); PSA_DONE(); } diff --git a/tests/suites/test_suite_psa_crypto_pake.function b/tests/suites/test_suite_psa_crypto_pake.function index 125c9e1245..303ef0deaf 100644 --- a/tests/suites/test_suite_psa_crypto_pake.function +++ b/tests/suites/test_suite_psa_crypto_pake.function @@ -763,8 +763,8 @@ void ecjpake_setup(int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, } exit: - PSA_ASSERT(psa_destroy_key(key)); - PSA_ASSERT(psa_pake_abort(&operation)); + psa_destroy_key(key); + psa_pake_abort(&operation); mbedtls_free(output_buffer); PSA_DONE(); } @@ -1035,8 +1035,8 @@ void pake_input_getters_password() TEST_MEMORY_COMPARE(password_ret, buffer_len_ret, password, strlen(password)); exit: - PSA_ASSERT(psa_destroy_key(key)); - PSA_ASSERT(psa_pake_abort(&operation)); + psa_destroy_key(key); + psa_pake_abort(&operation); PSA_DONE(); } /* END_CASE */ @@ -1071,7 +1071,7 @@ void pake_input_getters_cipher_suite() PSA_ERROR_BAD_STATE); exit: - PSA_ASSERT(psa_pake_abort(&operation)); + psa_pake_abort(&operation); PSA_DONE(); } /* END_CASE */ @@ -1133,8 +1133,9 @@ void pake_input_getters_user() TEST_MEMORY_COMPARE(user_ret, buffer_len_ret, user, user_len); } + exit: - PSA_ASSERT(psa_pake_abort(&operation)); + psa_pake_abort(&operation); PSA_DONE(); } /* END_CASE */ @@ -1196,8 +1197,9 @@ void pake_input_getters_peer() TEST_MEMORY_COMPARE(peer_ret, buffer_len_ret, peer, peer_len); } + exit: - PSA_ASSERT(psa_pake_abort(&operation)); + psa_pake_abort(&operation); PSA_DONE(); } /* END_CASE */