From 6c8f6597f95144ce3e365fdd1a56bfee42d7d0a5 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 17 Oct 2022 15:08:29 +0530 Subject: [PATCH 1/2] mbedtls: test_app: keep release config enabled for ESP32 Before `test_apps` migration, we had an independent release config, but we can safely enable it in the default configuration for ESP32 target itself. This helps to catch any potential issues that may occur in relevant tests because of compiler optimization flags. --- components/mbedtls/test_apps/main/test_esp_crt_bundle.c | 7 +++++++ components/mbedtls/test_apps/sdkconfig.defaults.esp32 | 1 + 2 files changed, 8 insertions(+) diff --git a/components/mbedtls/test_apps/main/test_esp_crt_bundle.c b/components/mbedtls/test_apps/main/test_esp_crt_bundle.c index 1080863105..a4b0af46a5 100644 --- a/components/mbedtls/test_apps/main/test_esp_crt_bundle.c +++ b/components/mbedtls/test_apps/main/test_esp_crt_bundle.c @@ -23,6 +23,7 @@ #include "mbedtls/net_sockets.h" #include "mbedtls/error.h" #include "mbedtls/debug.h" +#include "mbedtls/esp_debug.h" #include "esp_crt_bundle.h" #include "esp_random.h" @@ -91,6 +92,9 @@ esp_err_t server_setup(mbedtls_endpoint_t *server) { int ret; mbedtls_ssl_config_init( &server->conf ); +#if CONFIG_MBEDTLS_DEBUG + mbedtls_esp_enable_debug_log( &server->conf, CONFIG_MBEDTLS_DEBUG_LEVEL ); +#endif mbedtls_net_init( &server->listen_fd ); mbedtls_net_init( &server->client_fd ); mbedtls_ssl_init( &server->ssl ); @@ -213,6 +217,9 @@ esp_err_t client_setup(mbedtls_endpoint_t *client) { int ret; mbedtls_ssl_config_init( &client->conf ); +#if CONFIG_MBEDTLS_DEBUG + mbedtls_esp_enable_debug_log( &client->conf, CONFIG_MBEDTLS_DEBUG_LEVEL ); +#endif mbedtls_net_init( &client->client_fd ); mbedtls_ssl_init( &client->ssl ); mbedtls_x509_crt_init( &client->cert ); diff --git a/components/mbedtls/test_apps/sdkconfig.defaults.esp32 b/components/mbedtls/test_apps/sdkconfig.defaults.esp32 index 59093635f0..66cbe6577e 100644 --- a/components/mbedtls/test_apps/sdkconfig.defaults.esp32 +++ b/components/mbedtls/test_apps/sdkconfig.defaults.esp32 @@ -1,3 +1,4 @@ CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ_240=y CONFIG_XTAL_FREQ_AUTO=y CONFIG_SPI_FLASH_SHARE_SPI1_BUS=y +CONFIG_COMPILER_OPTIMIZATION_SIZE=y From dc34d4986adb58e4a4b3f3074738e2a114eacb47 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 17 Oct 2022 15:09:25 +0530 Subject: [PATCH 2/2] esp32: mpi: add workaround for data corruption issue observed with IDF 5.x toolchain This fix adds a workaround to disable compiler optimization flag "-ftree-loop-distribute-patterns" for `mpi_to_mem_block` routine. It was observed that compiler with release configuration was falling back to `memset` call from ROM library causing an issue in correctly zero initializing MPI peripheral block. Please see following linked issue for more discussion and context on this issue. Closes https://github.com/espressif/esp-idf/issues/8710 Closes https://github.com/espressif/esp-idf/issues/9371 Closes https://github.com/espressif/esp-idf/issues/9256 Closes IDFGH-7102 Closes IDFGH-7842 Closes IDFGH-7714 Closes IDFCI-1452 Closes IDF-6029 --- components/mbedtls/port/esp32/bignum.c | 27 +++++++++++++++++++++++++- components/mbedtls/port/esp_bignum.c | 7 +++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/components/mbedtls/port/esp32/bignum.c b/components/mbedtls/port/esp32/bignum.c index d94b255aa3..2b55bc4c7d 100644 --- a/components/mbedtls/port/esp32/bignum.c +++ b/components/mbedtls/port/esp32/bignum.c @@ -67,7 +67,12 @@ void esp_mpi_interrupt_clear( void ) these additional words will be zeroed in the memory buffer. */ -static inline void mpi_to_mem_block(uint32_t mem_base, const mbedtls_mpi *mpi, size_t hw_words) + +/* Please see detailed note inside the function body below. + * Relevant: https://github.com/espressif/esp-idf/issues/8710 and IDF-6029 + */ +static inline void __attribute__((optimize("-fno-tree-loop-distribute-patterns"))) + mpi_to_mem_block(uint32_t mem_base, const mbedtls_mpi *mpi, size_t hw_words) { uint32_t *pbase = (uint32_t *)mem_base; uint32_t copy_words = MIN(hw_words, mpi->MBEDTLS_PRIVATE(n)); @@ -81,6 +86,26 @@ static inline void mpi_to_mem_block(uint32_t mem_base, const mbedtls_mpi *mpi, s for (uint32_t i = copy_words; i < hw_words; i++) { pbase[i] = 0; } + +#if _INTERNAL_DEBUG_PURPOSE + /* + * With Xtensa GCC 11.2.0 (from ESP-IDF v5.x), it was observed that above zero initialization + * loop gets optimized to `memset` call from the ROM library. This was causing an issue that + * specific write (store) operation to the MPI peripheral block was getting lost erroneously. + * Following data re-verify loop could catch it during runtime. + * + * As a workaround, we are disabling loop distribute patterns for this function and hence + * compiler does not enforce usage of `memset` (or `memcpy`) calls for this routine. It + * appears that `-ftree-loop-distribute-patterns` was enabled with O2/Os starting from + * GCC-10.x. It is quite possible that there is some issue with DPORT write with sequence of + * store instructions as generated by `memset` call, but for now this should serve as good + * interim workaround without any impact on the performance. + * + * Please see IDF-6029 for more details. + */ + + //for (uint32_t i = copy_words; i < hw_words; i++) { assert(pbase[i] == 0); } +#endif } /* Read mbedTLS MPI bignum back from hardware memory block. diff --git a/components/mbedtls/port/esp_bignum.c b/components/mbedtls/port/esp_bignum.c index 96601e056c..0cc987cbfa 100644 --- a/components/mbedtls/port/esp_bignum.c +++ b/components/mbedtls/port/esp_bignum.c @@ -642,6 +642,13 @@ static int mpi_mult_mpi_failover_mod_mult( mbedtls_mpi *Z, const mbedtls_mpi *X, esp_mpi_read_result_hw_op(Z, hw_words); Z->MBEDTLS_PRIVATE(s) = X->MBEDTLS_PRIVATE(s) * Y->MBEDTLS_PRIVATE(s); + /* + * If this condition fails then most likely hardware peripheral + * has produced an incorrect result for MPI operation. This can + * happen if data fed to the peripheral register was incorrect. + * Relevant: https://github.com/espressif/esp-idf/issues/8710#issuecomment-1249178698 + */ + assert(mpi_words(Z) == z_words); cleanup: esp_mpi_disable_hardware_hw_op(); return ret;