From 8ee7d381ecf06e1003981757b0de26996d4731b0 Mon Sep 17 00:00:00 2001 From: gojimmypi Date: Fri, 11 Apr 2025 19:37:55 +0200 Subject: [PATCH] Fix hash_test() memory leak in wolfcrypt/test/test.c (#8506) * Fix hash_test() memory leak in wolfcrypt/test/test.c * Escape HASH_TYPE_E comparisons * Revised hash_test() in test.c * Use ERROR_OUT and WC_NO_ERR_TRACE patterns, polish * Remove placeholder init, no longer needed * remove verbose hash_test() WOLFSSL_MSG and PRINT_HEAP_CHECKPOINT --- .../wolfssl/include/user_settings.h | 3 - .../ESP-IDF/examples/wolfssl_test/main/main.c | 8 +- wolfcrypt/test/test.c | 135 ++++++++++++++++-- .../wolfcrypt/port/Espressif/esp32-crypt.h | 9 +- 4 files changed, 135 insertions(+), 20 deletions(-) diff --git a/IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/include/user_settings.h b/IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/include/user_settings.h index 8127cd167..89498ae70 100644 --- a/IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/include/user_settings.h +++ b/IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/include/user_settings.h @@ -559,9 +559,6 @@ defined(WOLFSSL_SP_RISCV32) #endif -#define WOLFSSL_SMALL_STACK - - #define HAVE_VERSION_EXTENDED_INFO /* #define HAVE_WC_INTROSPECTION */ diff --git a/IDE/Espressif/ESP-IDF/examples/wolfssl_test/main/main.c b/IDE/Espressif/ESP-IDF/examples/wolfssl_test/main/main.c index 88480f552..32e0bb895 100644 --- a/IDE/Espressif/ESP-IDF/examples/wolfssl_test/main/main.c +++ b/IDE/Espressif/ESP-IDF/examples/wolfssl_test/main/main.c @@ -164,6 +164,8 @@ void app_main(void) .stop_bits = UART_STOP_BITS_1, }; int stack_start = 0; + int heap_start = 0; + int heap_current = 0; int loops = 0; esp_err_t ret = 0; @@ -245,8 +247,12 @@ void app_main(void) ** note it is still always called in wolf_test_task. */ stack_start = uxTaskGetStackHighWaterMark(NULL); + heap_start = heap_caps_get_free_size(MALLOC_CAP_8BIT); do { + heap_current = heap_caps_get_free_size(MALLOC_CAP_8BIT); + ESP_LOGI(TAG, "Free heap memory: %d bytes; Start %d", + heap_current, heap_start); ESP_LOGI(TAG, "Stack HWM: %d\n", uxTaskGetStackHighWaterMark(NULL)); ret = wolf_test_task(); @@ -272,7 +278,7 @@ void app_main(void) ESP_LOGI(TAG, "Stack HWM: %d\n", uxTaskGetStackHighWaterMark(NULL)); #endif -#if defined(DEBUG_WOLFSSL) && defined(WOLFSSL_ESP32_CRYPT_RSA_PRI) +#if defined(WOLFSSL_HW_METRICS) && defined(WOLFSSL_ESP32_CRYPT_RSA_PRI) esp_hw_show_mp_metrics(); #endif diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 6147323f1..bb85a3f4a 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -92,7 +92,9 @@ const byte const_byte_array[] = "A+Gd\0\0\0"; #else static ssize_t max_relative_heap_bytes = -1; #endif -#define PRINT_HEAP_CHECKPOINT() { \ + +/* Optional breadcrumb string (b), and interaction, (i) not implemented */ +#define PRINT_HEAP_CHECKPOINT(b, i) { \ const ssize_t _rha = wolfCrypt_heap_peakAllocs_checkpoint() - heap_baselineAllocs; \ const ssize_t _rhb = wolfCrypt_heap_peakBytes_checkpoint() - heap_baselineBytes; \ printf(" relative heap peak usage: %ld alloc%s, %ld bytes\n", \ @@ -109,9 +111,54 @@ const byte const_byte_array[] = "A+Gd\0\0\0"; heap_baselineBytes = wolfCrypt_heap_peakBytes_checkpoint(); \ } #else -#define PRINT_HEAP_CHECKPOINT() WC_DO_NOTHING + #define PRINT_HEAP_CHECKPOINT(b, i) WC_DO_NOTHING; + #define PRINT_HEAP_ADDRESS(p) WC_DO_NOTHING; #endif /* WOLFSSL_TRACK_MEMORY_VERBOSE && !WOLFSSL_STATIC_MEMORY */ +#ifdef WOLFSSL_ESPIDF + #undef PRINT_HEAP_CHECKPOINT + #undef PRINT_HEAP_ADDRESS + static int esp_start_heap = 0; + static int esp_last_heap = 0; + static int esp_this_heap = 0; + + #ifdef DEBUG_WOLFSSL_ESP32_HEAP + #define PRINT_HEAP_CHECKPOINT(b, i) \ + esp_last_heap = esp_this_heap; \ + esp_this_heap = (int)heap_caps_get_free_size(MALLOC_CAP_8BIT); \ + if (esp_start_heap == 0) { \ + esp_start_heap = esp_this_heap; \ + } \ + ESP_LOGI(ESPIDF_TAG, "%s #%d; Heap free: %d", \ + ((b) ? (b) : ""), /* breadcumb string */ \ + ((i) ? (i) : 0), /* index */ \ + esp_this_heap); + + #define PRINT_HEAP_ADDRESS(p) \ + ESP_LOGI(ESPIDF_TAG, "Allocated address: %p", (void *)(p)); + #else + /* Even without verbose heap, we'll warn on anomalous values */ + #define PRINT_HEAP_CHECKPOINT(b, i) \ + esp_last_heap = esp_this_heap; \ + esp_this_heap = (int)heap_caps_get_free_size(MALLOC_CAP_8BIT); \ + if (esp_start_heap == 0) { \ + esp_start_heap = esp_this_heap; \ + esp_last_heap = esp_this_heap; \ + } \ + if (esp_this_heap == esp_last_heap) { \ + ESP_LOGV(ESPIDF_TAG, "Heap constant: %d", esp_this_heap); \ + } \ + else { \ + ESP_LOGI(ESPIDF_TAG, "Breadcrumb: %s", ((b) ? (b) : "")); \ + ESP_LOGW(ESPIDF_TAG, "Warning: this heap %d != last %d", \ + esp_this_heap, esp_last_heap); \ + } + + #define PRINT_HEAP_ADDRESS(p) WC_DO_NOTHING; + #endif +#endif /* WOLFSSL_ESPIDF */ + + #ifdef USE_FLAT_TEST_H #ifdef HAVE_CONFIG_H #include "test_paths.h" @@ -1415,7 +1462,7 @@ static WOLFSSL_TEST_SUBROUTINE wc_test_ret_t nist_sp80056c_kdf_test(void) va_start(args, fmt); STACK_SIZE_CHECKPOINT_WITH_MAX_CHECK(max_relative_stack, vprintf(fmt, args)); va_end(args); - PRINT_HEAP_CHECKPOINT(); + PRINT_HEAP_CHECKPOINT("",0); TEST_SLEEP(); ASSERT_RESTORED_VECTOR_REGISTERS(exit(1);); } @@ -1429,7 +1476,7 @@ static WOLFSSL_TEST_SUBROUTINE wc_test_ret_t nist_sp80056c_kdf_test(void) (max_relative_stack, printf(__VA_ARGS__)) < 0) { \ return err_sys("post-test check failed", WC_TEST_RET_ENC_NC);\ } \ - PRINT_HEAP_CHECKPOINT(); \ + PRINT_HEAP_CHECKPOINT("TEST_PASS", 0) \ ASSERT_RESTORED_VECTOR_REGISTERS(exit(1);); \ } #endif @@ -6156,6 +6203,7 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t hash_test(void) #else wc_HashAlg hash[1]; #endif + int ret, exp_ret; int i, j; int digestSz; @@ -6216,7 +6264,10 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t hash_test(void) #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) hash = wc_HashNew(WC_HASH_TYPE_SHA256, HEAP_HINT, devId, &ret); if (hash == NULL) { - return WC_TEST_RET_ENC_EC(ret); + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), out); + } + else { + PRINT_HEAP_ADDRESS(hash); } #else XMEMSET(hash, 0, sizeof(wc_HashAlg)); @@ -6245,9 +6296,23 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t hash_test(void) if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) ERROR_OUT(WC_TEST_RET_ENC_EC(ret), out); +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + /* Delete the WC_HASH_TYPE_SHA256 type hash for the following tests */ + ret = wc_HashDelete(hash, &hash); + if (ret != 0) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), out); +#endif + /* Try invalid hash algorithms. */ for (i = 0; i < (int)(sizeof(typesBad)/sizeof(*typesBad)); i++) { +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + hash = wc_HashNew(typesBad[i], HEAP_HINT, devId, &ret); +#endif + if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) { + ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + } ret = wc_HashInit(hash, typesBad[i]); + if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); ret = wc_HashUpdate(hash, typesBad[i], data, sizeof(data)); @@ -6256,17 +6321,51 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t hash_test(void) ret = wc_HashFinal(hash, typesBad[i], out); if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); - wc_HashFree(hash, typesBad[i]); + ret = wc_HashFree(hash, typesBad[i]); + if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + ret = wc_HashDelete(hash, &hash); + if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) { + WOLFSSL_MSG("ERROR: wc_HashDelete failed, expected BAD_FUNC_ARG."); + ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + } +#endif } /* Try valid hash algorithms. */ - for (i = 0, j = 0; i < (int)(sizeof(typesGood)/sizeof(*typesGood)); i++) { - exp_ret = 0; - if (typesGood[i] == typesNoImpl[j]) { - /* Recognized but no implementation compiled in. */ - exp_ret = HASH_TYPE_E; - j++; + for (i = 0; i < (int)(sizeof(typesGood)/sizeof(*typesGood)); i++) { + exp_ret = 0; /* For valid had, we expect return result to be zero */ + + /* See if the current hash type is one of the known types that are + * not implemented or not compiled in (disabled): */ + for(j = 0; j < (int)(sizeof(typesNoImpl) / sizeof(*typesNoImpl)); j++) { + if (typesGood[i] == typesNoImpl[j]) { + exp_ret = HASH_TYPE_E; + break; /* found one. don't keep looking. + * we won't test hashes not implemented */ + } } + + /* If the expected return value is HASH_TYPE_E before we've even started + * it must be a hash type not implemented or disabled, so skip it. */ + if (exp_ret == WC_NO_ERR_TRACE(HASH_TYPE_E)) { + continue; /* go fetch the next typesGood[i] */ + } + + /* Good type and implemented: */ +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + hash = wc_HashNew(typesGood[i], HEAP_HINT, devId, &ret); + if (hash == NULL) { + WOLFSSL_MSG("ERROR: wc_HashNew failed."); + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), out); + } + + if (ret != 0) { + ERROR_OUT(WC_TEST_RET_ENC_I(BAD_FUNC_ARG), out); + } +#endif ret = wc_HashInit(hash, typesGood[i]); if (ret != exp_ret) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); @@ -6313,8 +6412,18 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t hash_test(void) if (exp_ret == 0 && hashType != typesGood[i]) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); #endif /* !defined(NO_ASN) || !defined(NO_DH) || defined(HAVE_ECC) */ - } +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + ret = wc_HashDelete(hash, &hash); + if (ret < 0) { + WOLFSSL_MSG("ERROR: Failed to delete hash."); + ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + } +#endif + } /* Valid hash functions */ + + + /* non wc_HashAlg hash object tests follow: */ for (i = 0; i < (int)(sizeof(typesHashBad)/sizeof(*typesHashBad)); i++) { ret = wc_Hash(typesHashBad[i], data, sizeof(data), out, sizeof(out)); if ((ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) && diff --git a/wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h b/wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h index a74d796f1..ac48d9773 100644 --- a/wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h +++ b/wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h @@ -216,8 +216,11 @@ enum { ** Turns on diagnostic messages for SHA mutex. Note that given verbosity, ** there may be TLS timing issues encountered. Use with caution. ** +** DEBUG_WOLFSSL_ESP32_HEAP +** Prints heap memory usage +** ** DEBUG_WOLFSSL_ESP32_UNFINISHED_HW -** This may be interesting in that HW may have been unnessearily locked +** This may be interesting in that HW may have been unnecessarily locked ** for hash that was never completed. (typically encountered at `free1` time) ** ** LOG_LOCAL_LEVEL @@ -234,11 +237,11 @@ enum { ** Shows a warning when mulm falls back for minimum number of bits. ** ** WOLFSSL_DEBUG_ESP_HW_MULTI_RSAMAX_BITS -** Shows a marning when multiplication math bits have exceeded hardware +** Shows a warning when multiplication math bits have exceeded hardware ** capabilities and will fall back to slower software. ** ** WOLFSSL_DEBUG_ESP_HW_MOD_RSAMAX_BITS -** Shows a marning when modular math bits have exceeded hardware capabilities +** Shows a warning when modular math bits have exceeded hardware capabilities ** and will fall back to slower software. ** ** NO_HW_MATH_TEST