From 89584cd1d05a674ae23610c12abf4d4f896b4848 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Fri, 1 Sep 2023 17:17:12 +0530 Subject: [PATCH 1/2] fix(aes-gcm): correct the DMA completion wait condition for hardware GCM case DMA operation completion must wait until the last DMA descriptor ownership has been changed to hardware, that is hardware is completed the write operation for entire data. Earlier for the hardware GCM case, the first DMA descriptor was checked and it could have resulted in some race condition for non interrupt (MBEDTLS_AES_USE_INTERRUPT disabled) case. --- components/mbedtls/port/aes/dma/esp_aes.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/mbedtls/port/aes/dma/esp_aes.c b/components/mbedtls/port/aes/dma/esp_aes.c index 2e3e61942ac7..92c53452a6b5 100644 --- a/components/mbedtls/port/aes/dma/esp_aes.c +++ b/components/mbedtls/port/aes/dma/esp_aes.c @@ -495,6 +495,7 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input, int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, unsigned char *output, size_t len, lldesc_t *aad_desc, size_t aad_len) { lldesc_t *in_desc_head = NULL, *out_desc_head = NULL, *len_desc = NULL; + lldesc_t *out_desc_tail = NULL; /* pointer to the final output descriptor */ lldesc_t stream_in_desc, stream_out_desc; lldesc_t *block_desc = NULL, *block_in_desc = NULL, *block_out_desc = NULL; size_t lldesc_num; @@ -544,6 +545,8 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un lldesc_append(&in_desc_head, block_in_desc); lldesc_append(&out_desc_head, block_out_desc); + + out_desc_tail = &block_out_desc[lldesc_num - 1]; } /* Any leftover bytes which are appended as an additional DMA list */ @@ -555,6 +558,8 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un lldesc_append(&in_desc_head, &stream_in_desc); lldesc_append(&out_desc_head, &stream_out_desc); + + out_desc_tail = &stream_out_desc; } @@ -593,7 +598,7 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un aes_hal_transform_dma_gcm_start(blocks); - if (esp_aes_dma_wait_complete(use_intr, out_desc_head) < 0) { + if (esp_aes_dma_wait_complete(use_intr, out_desc_tail) < 0) { ESP_LOGE(TAG, "esp_aes_dma_wait_complete failed"); ret = -1; goto cleanup; From 9dc4b8beeb1a8bb2bf139ab201f922633133ee98 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Fri, 1 Sep 2023 13:46:22 +0530 Subject: [PATCH 2/2] fix(aes): correct the linking of the DMA descriptors For certain data lengths, the last input descriptor was not getting appended correctly and hence the EOF flag in the DMA descriptor link list was set at incorrect location. This was resulting in the peripheral being stalled expecting more data and eventually the code used to timeout waiting for the AES completion interrupt. Required configs for this issue: CONFIG_MBEDTLS_HARDWARE_AES CONFIG_SOC_AES_SUPPORT_DMA This observation is similar to the issue reported in: https://github.com/espressif/esp-idf/issues/10647 To recreate this issue, start the AES-GCM DMA operation with data length 12280 bytes and this should stall the operation forever. In this fix, we are tracing the entire descriptor list and then appending the extra bytes descriptor at correct position (as the last node). --- components/mbedtls/port/aes/dma/esp_aes.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/components/mbedtls/port/aes/dma/esp_aes.c b/components/mbedtls/port/aes/dma/esp_aes.c index 92c53452a6b5..896980569887 100644 --- a/components/mbedtls/port/aes/dma/esp_aes.c +++ b/components/mbedtls/port/aes/dma/esp_aes.c @@ -400,6 +400,10 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input, //Limit max inlink descriptor length to be 16 byte aligned, require for EDMA lldesc_setup_link_constrained(block_out_desc, output, block_bytes, LLDESC_MAX_NUM_PER_DESC_16B_ALIGNED, 0); + /* Setup in/out start descriptors */ + lldesc_append(&in_desc_head, block_in_desc); + lldesc_append(&out_desc_head, block_out_desc); + out_desc_tail = &block_out_desc[lldesc_num - 1]; } @@ -417,20 +421,13 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input, lldesc_setup_link(&s_stream_in_desc, s_stream_in, AES_BLOCK_BYTES, 0); lldesc_setup_link(&s_stream_out_desc, s_stream_out, AES_BLOCK_BYTES, 0); - if (block_bytes > 0) { - /* Link with block descriptors*/ - block_in_desc[lldesc_num - 1].empty = (uint32_t)&s_stream_in_desc; - block_out_desc[lldesc_num - 1].empty = (uint32_t)&s_stream_out_desc; - } + /* Link with block descriptors */ + lldesc_append(&in_desc_head, &s_stream_in_desc); + lldesc_append(&out_desc_head, &s_stream_out_desc); out_desc_tail = &s_stream_out_desc; } - // block buffers are sent to DMA first, unless there aren't any - in_desc_head = (block_bytes > 0) ? block_in_desc : &s_stream_in_desc; - out_desc_head = (block_bytes > 0) ? block_out_desc : &s_stream_out_desc; - - #if defined (CONFIG_MBEDTLS_AES_USE_INTERRUPT) /* Only use interrupt for long AES operations */ if (len > AES_DMA_INTR_TRIG_LEN) {