From d3be7bda056dfcb7edbac4cf8757ec1ac7d0a1ea Mon Sep 17 00:00:00 2001 From: "harshal.patil" Date: Fri, 20 Oct 2023 11:32:35 +0530 Subject: [PATCH] fix(mbedtls): move interrupt allocation during initialization phase --- components/esp-tls/test_apps/main/app_main.c | 15 +++++++-- components/mbedtls/port/aes/dma/esp_aes.c | 31 ++++++++++++------- .../port/aes/dma/include/esp_aes_dma_priv.h | 23 ++++++-------- components/mbedtls/port/aes/esp_aes_common.c | 13 +++++--- components/mbedtls/port/bignum/esp_bignum.c | 17 +++++++--- components/mbedtls/test_apps/main/app_main.c | 11 ++++++- tools/ci/check_copyright_ignore.txt | 1 - 7 files changed, 72 insertions(+), 39 deletions(-) diff --git a/components/esp-tls/test_apps/main/app_main.c b/components/esp-tls/test_apps/main/app_main.c index 401a39d745..d84480083f 100644 --- a/components/esp-tls/test_apps/main/app_main.c +++ b/components/esp-tls/test_apps/main/app_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,6 +7,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "unity.h" +#include "mbedtls/aes.h" #include "memory_checks.h" #include "soc/soc_caps.h" #if SOC_SHA_SUPPORT_PARALLEL_ENG @@ -26,13 +27,21 @@ /* setUp runs before every test */ void setUp(void) { - // Execute esp_sha operation to allocate internal SHA semaphore memory - // which is considered as leaked otherwise + // Execute esp_sha operation to allocate internal SHA semaphore (in case of ESP32) + // and initial DMA setup memory which is considered as leaked otherwise #if SOC_SHA_SUPPORTED const uint8_t input_buffer[64] = {0}; uint8_t output_buffer[64]; esp_sha(SHA_TYPE, input_buffer, sizeof(input_buffer), output_buffer); #endif // SOC_SHA_SUPPORTED + + // Execute mbedtls_aes_init operation to allocate AES interrupt + // allocation memory which is considered as leak otherwise +#if SOC_AES_SUPPORTED + mbedtls_aes_context ctx; + mbedtls_aes_init(&ctx); +#endif // SOC_AES_SUPPORTED + test_utils_record_free_mem(); TEST_ESP_OK(test_utils_set_leak_level(0, ESP_LEAK_TYPE_CRITICAL, ESP_COMP_LEAK_GENERAL)); TEST_ESP_OK(test_utils_set_leak_level(0, ESP_LEAK_TYPE_WARNING, ESP_COMP_LEAK_GENERAL)); diff --git a/components/mbedtls/port/aes/dma/esp_aes.c b/components/mbedtls/port/aes/dma/esp_aes.c index 2b0e8b3fda..693586813b 100644 --- a/components/mbedtls/port/aes/dma/esp_aes.c +++ b/components/mbedtls/port/aes/dma/esp_aes.c @@ -177,24 +177,31 @@ static IRAM_ATTR void esp_aes_complete_isr(void *arg) } } -static esp_err_t esp_aes_isr_initialise( void ) +void esp_aes_intr_alloc(void) { - aes_hal_interrupt_clear(); - aes_hal_interrupt_enable(true); if (op_complete_sem == NULL) { - op_complete_sem = xSemaphoreCreateBinary(); - - if (op_complete_sem == NULL) { - ESP_LOGE(TAG, "Failed to create intr semaphore"); - return ESP_FAIL; - } const int isr_flags = esp_intr_level_to_flags(CONFIG_MBEDTLS_AES_INTERRUPT_LEVEL); esp_err_t ret = esp_intr_alloc(ETS_AES_INTR_SOURCE, isr_flags, esp_aes_complete_isr, NULL, NULL); if (ret != ESP_OK) { - return ret; + ESP_LOGE(TAG, "Failed to allocate AES interrupt %d", ret); + // This should be treated as fatal error as this API would mostly + // be invoked within mbedTLS interface. There is no way for the system + // to proceed if the AES interrupt allocation fails here. + abort(); } + + static StaticSemaphore_t op_sem_buf; + op_complete_sem = xSemaphoreCreateBinaryStatic(&op_sem_buf); + // Static semaphore creation is unlikley to fail but still basic sanity + assert(op_complete_sem != NULL); } +} + +static esp_err_t esp_aes_isr_initialise( void ) +{ + aes_hal_interrupt_clear(); + aes_hal_interrupt_enable(true); /* AES is clocked proportionally to CPU clock, take power management lock */ #ifdef CONFIG_PM_ENABLE @@ -433,7 +440,7 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input, /* Only use interrupt for long AES operations */ if (len > AES_DMA_INTR_TRIG_LEN) { use_intr = true; - if (esp_aes_isr_initialise() == ESP_FAIL) { + if (esp_aes_isr_initialise() != ESP_OK) { ESP_LOGE(TAG, "ESP-AES ISR initialisation failed"); ret = -1; goto cleanup; @@ -576,7 +583,7 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un /* Only use interrupt for long AES operations */ if (len > AES_DMA_INTR_TRIG_LEN) { use_intr = true; - if (esp_aes_isr_initialise() == ESP_FAIL) { + if (esp_aes_isr_initialise() != ESP_OK) { ESP_LOGE(TAG, "ESP-AES ISR initialisation failed"); ret = -1; goto cleanup; diff --git a/components/mbedtls/port/aes/dma/include/esp_aes_dma_priv.h b/components/mbedtls/port/aes/dma/include/esp_aes_dma_priv.h index 7880f18806..d8ddac8d28 100644 --- a/components/mbedtls/port/aes/dma/include/esp_aes_dma_priv.h +++ b/components/mbedtls/port/aes/dma/include/esp_aes_dma_priv.h @@ -1,16 +1,8 @@ -// Copyright 2020 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #pragma once @@ -43,6 +35,11 @@ esp_err_t esp_aes_dma_start(const lldesc_t *input, const lldesc_t *output); */ bool esp_aes_dma_done(const lldesc_t *output); +/** + * @brief Allocate AES peripheral interrupt handler + */ +void esp_aes_intr_alloc(void); + #ifdef __cplusplus } #endif diff --git a/components/mbedtls/port/aes/esp_aes_common.c b/components/mbedtls/port/aes/esp_aes_common.c index 5af90a8003..578ec6656c 100644 --- a/components/mbedtls/port/aes/esp_aes_common.c +++ b/components/mbedtls/port/aes/esp_aes_common.c @@ -6,7 +6,7 @@ * * SPDX-License-Identifier: Apache-2.0 * - * SPDX-FileContributor: 2016-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileContributor: 2016-2023 Espressif Systems (Shanghai) CO LTD */ /* * The AES block cipher was designed by Vincent Rijmen and Joan Daemen. @@ -14,6 +14,7 @@ * http://csrc.nist.gov/encryption/aes/rijndael/Rijndael.pdf * http://csrc.nist.gov/publications/fips/fips197/fips-197.pdf */ +#include "sdkconfig.h" #include "aes/esp_aes_internal.h" #include "mbedtls/aes.h" #include "hal/aes_hal.h" @@ -24,7 +25,7 @@ #include #include "mbedtls/platform.h" -#if SOC_AES_GDMA +#if SOC_AES_SUPPORT_DMA #include "esp_aes_dma_priv.h" #endif @@ -39,10 +40,12 @@ bool valid_key_length(const esp_aes_context *ctx) return valid_len; } - -void esp_aes_init( esp_aes_context *ctx ) +void esp_aes_init(esp_aes_context *ctx) { - bzero( ctx, sizeof( esp_aes_context ) ); + bzero(ctx, sizeof(esp_aes_context)); +#if SOC_AES_SUPPORT_DMA && CONFIG_MBEDTLS_AES_USE_INTERRUPT + esp_aes_intr_alloc(); +#endif } void esp_aes_free( esp_aes_context *ctx ) diff --git a/components/mbedtls/port/bignum/esp_bignum.c b/components/mbedtls/port/bignum/esp_bignum.c index 1611f88e12..bda0baf8c0 100644 --- a/components/mbedtls/port/bignum/esp_bignum.c +++ b/components/mbedtls/port/bignum/esp_bignum.c @@ -78,8 +78,8 @@ static esp_err_t esp_mpi_isr_initialise(void) mpi_hal_clear_interrupt(); mpi_hal_interrupt_enable(true); if (op_complete_sem == NULL) { - op_complete_sem = xSemaphoreCreateBinary(); - + static StaticSemaphore_t op_sem_buf; + op_complete_sem = xSemaphoreCreateBinaryStatic(&op_sem_buf); if (op_complete_sem == NULL) { ESP_LOGE(TAG, "Failed to create intr semaphore"); return ESP_FAIL; @@ -87,7 +87,16 @@ static esp_err_t esp_mpi_isr_initialise(void) const int isr_flags = esp_intr_level_to_flags(CONFIG_MBEDTLS_MPI_INTERRUPT_LEVEL); - esp_intr_alloc(ETS_RSA_INTR_SOURCE, isr_flags, esp_mpi_complete_isr, NULL, NULL); + esp_err_t ret; + ret = esp_intr_alloc(ETS_RSA_INTR_SOURCE, isr_flags, esp_mpi_complete_isr, NULL, NULL); + if (ret != ESP_OK) { + ESP_LOGE(TAG, "Failed to allocate RSA interrupt %d", ret); + + // This should be treated as fatal error as this API would mostly + // be invoked within mbedTLS interface. There is no way for the system + // to proceed if the MPI interrupt allocation fails here. + abort(); + } } /* MPI is clocked proportionally to CPU clock, take power management lock */ @@ -400,7 +409,7 @@ static int esp_mpi_exp_mod( mbedtls_mpi *Z, const mbedtls_mpi *X, const mbedtls_ esp_mpi_enable_hardware_hw_op(); #if defined (CONFIG_MBEDTLS_MPI_USE_INTERRUPT) - if (esp_mpi_isr_initialise() == ESP_FAIL) { + if (esp_mpi_isr_initialise() != ESP_OK) { ret = -1; esp_mpi_disable_hardware_hw_op(); goto cleanup; diff --git a/components/mbedtls/test_apps/main/app_main.c b/components/mbedtls/test_apps/main/app_main.c index 0716defabf..27735fb98d 100644 --- a/components/mbedtls/test_apps/main/app_main.c +++ b/components/mbedtls/test_apps/main/app_main.c @@ -1,16 +1,25 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "unity.h" +#include "mbedtls/aes.h" #include "memory_checks.h" +#include "soc/soc_caps.h" /* setUp runs before every test */ void setUp(void) { + // Execute mbedtls_aes_init operation to allocate AES interrupt + // allocation memory which is considered as leak otherwise +#if SOC_AES_SUPPORTED + mbedtls_aes_context ctx; + mbedtls_aes_init(&ctx); +#endif // SOC_AES_SUPPORTED + test_utils_record_free_mem(); test_utils_set_leak_level(CONFIG_UNITY_CRITICAL_LEAK_LEVEL_GENERAL, ESP_LEAK_TYPE_CRITICAL, ESP_COMP_LEAK_GENERAL); test_utils_set_leak_level(CONFIG_UNITY_WARN_LEAK_LEVEL_GENERAL, ESP_LEAK_TYPE_WARNING, ESP_COMP_LEAK_GENERAL); diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index 0dd14e756d..e40f85142f 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -561,7 +561,6 @@ components/mbedtls/esp_crt_bundle/test_gen_crt_bundle/test_gen_crt_bundle.py components/mbedtls/port/aes/block/esp_aes.c components/mbedtls/port/aes/dma/esp_aes.c components/mbedtls/port/aes/dma/esp_aes_crypto_dma_impl.c -components/mbedtls/port/aes/dma/include/esp_aes_dma_priv.h components/mbedtls/port/aes/esp_aes_xts.c components/mbedtls/port/include/aes/esp_aes.h components/mbedtls/port/include/aes/esp_aes_internal.h