fix(usb/host): Do not call heap_caps_get_allocated_size() in USB host driver

It causes heap corruption if heap poisoning is enabled on ESP32-P4.
It returns incorrect size on ESP32-S3.

Closes https://github.com/espressif/esp-idf/issues/15815
This commit is contained in:
Tomas Rezucha
2025-04-23 08:42:17 +02:00
parent 2423cd3257
commit 462c1ed957
6 changed files with 48 additions and 14 deletions

View File

@ -16,21 +16,24 @@
#include "esp_err.h"
#include "esp_log.h"
#include "soc/soc_caps.h"
#include "soc/usb_dwc_periph.h"
#include "hal/usb_dwc_hal.h"
#include "hcd.h"
#include "usb_private.h"
#include "usb/usb_types_ch9.h"
#include "soc/soc_caps.h"
#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
#include "esp_cache.h"
#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
#include "esp_private/esp_cache_private.h"
// ----------------------------------------------------- Macros --------------------------------------------------------
#define ALIGN_UP(num, align) ((align) == 0 ? (num) : (((num) + ((align) - 1)) & ~((align) - 1)))
// --------------------- Constants -------------------------
#define XFER_DESC_LIST_CAPS (MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED | MALLOC_CAP_INTERNAL)
#define INIT_DELAY_MS 30 // A delay of at least 25ms to enter Host mode. Make it 30ms to be safe
#define DEBOUNCE_DELAY_MS CONFIG_USB_HOST_DEBOUNCE_DELAY_MS
#define RESET_HOLD_MS CONFIG_USB_HOST_RESET_HOLD_MS
@ -1511,16 +1514,18 @@ static dma_buffer_block_t *buffer_block_alloc(usb_transfer_type_t type)
}
// Transfer descriptor list: Must be 512 aligned and DMA capable (USB-DWC requirement) and its size must be cache aligned
void *xfer_desc_list = heap_caps_aligned_calloc(USB_DWC_QTD_LIST_MEM_ALIGN, desc_list_len * sizeof(usb_dwc_ll_dma_qtd_t), 1, MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED | MALLOC_CAP_INTERNAL);
void *xfer_desc_list = heap_caps_aligned_calloc(USB_DWC_QTD_LIST_MEM_ALIGN, desc_list_len * sizeof(usb_dwc_ll_dma_qtd_t), 1, XFER_DESC_LIST_CAPS);
if (xfer_desc_list == NULL) {
free(buffer);
heap_caps_free(xfer_desc_list);
return NULL;
}
buffer->xfer_desc_list = xfer_desc_list;
// For targets with L1CACHE, the allocated size might be bigger than requested, this value is than used during memory sync
// We save this value here, so we don't have to call 'heap_caps_get_allocated_size()' during every memory sync
buffer->xfer_desc_list_len_bytes = heap_caps_get_allocated_size(xfer_desc_list);
// Note for developers: We do not use heap_caps_get_allocated_size() because it is broken with HEAP_POISONING=COMPREHENSIVE
size_t cache_align = 0;
esp_cache_get_alignment(XFER_DESC_LIST_CAPS, &cache_align);
buffer->xfer_desc_list_len_bytes = ALIGN_UP(desc_list_len * sizeof(usb_dwc_ll_dma_qtd_t), cache_align);
return buffer;
}

View File

@ -569,6 +569,7 @@ esp_err_t usb_host_endpoint_clear(usb_device_handle_t dev_hdl, uint8_t bEndpoint
*
* - This function allocates a transfer object
* - Each transfer object has a fixed sized buffer specified on allocation
* - The resulting data_buffer_size can be bigger that the requested size. This is to ensure that the data buffer is cache aligned
* - A transfer object can be re-used indefinitely
* - A transfer can be submitted using usb_host_transfer_submit() or usb_host_transfer_submit_control()
*

View File

@ -76,7 +76,8 @@ typedef bool (*usb_proc_req_cb_t)(usb_proc_req_source_t source, bool in_isr, voi
*
* - Data buffer is allocated in DMA capable memory
* - The constant fields of the URB are also set
* - The data_buffer field of the URB is set to point to start of the allocated data buffer.
* - The data_buffer field of the URB is set to point to start of the allocated data buffer
* - The resulting data_buffer_size can be bigger that the requested size. This is to ensure that the data buffer is cache aligned
*
* @param[in] data_buffer_size Size of the URB's data buffer
* @param[in] num_isoc_packets Number of isochronous packet descriptors

View File

@ -3,4 +3,6 @@ idf_component_register(SRCS "dev_hid.c"
"dev_msc.c"
"mock_msc.c"
INCLUDE_DIRS "."
REQUIRES usb unity)
REQUIRES usb unity
PRIV_REQUIRES esp_mm
)

View File

@ -278,18 +278,27 @@ void test_hcd_pipe_free(hcd_pipe_handle_t pipe_hdl)
vQueueDelete(pipe_evt_queue);
}
#include "esp_private/esp_cache_private.h"
#define DATA_BUFFER_CAPS (MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED)
#define ALIGN_UP(num, align) ((align) == 0 ? (num) : (((num) + ((align) - 1)) & ~((align) - 1)))
urb_t *test_hcd_alloc_urb(int num_isoc_packets, size_t data_buffer_size)
{
// Allocate a URB and data buffer
urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (sizeof(usb_isoc_packet_desc_t) * num_isoc_packets), MALLOC_CAP_DEFAULT);
void *data_buffer = heap_caps_malloc(data_buffer_size, MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED);
size_t cache_align = 0;
esp_cache_get_alignment(DATA_BUFFER_CAPS, &cache_align);
data_buffer_size = ALIGN_UP(data_buffer_size, cache_align);
void *data_buffer = heap_caps_malloc(data_buffer_size, DATA_BUFFER_CAPS);
TEST_ASSERT_NOT_NULL_MESSAGE(urb, "Failed to allocate URB");
TEST_ASSERT_NOT_NULL_MESSAGE(data_buffer, "Failed to allocate transfer buffer");
// Initialize URB and underlying transfer structure. Need to cast to dummy due to const fields
usb_transfer_dummy_t *transfer_dummy = (usb_transfer_dummy_t *)&urb->transfer;
transfer_dummy->data_buffer = data_buffer;
transfer_dummy->data_buffer_size = heap_caps_get_allocated_size(data_buffer);
transfer_dummy->data_buffer_size = data_buffer_size;
transfer_dummy->num_isoc_packets = num_isoc_packets;
return urb;
}

View File

@ -1,24 +1,40 @@
/*
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include "sdkconfig.h"
#include "esp_heap_caps.h"
#include "usb_private.h"
#include "usb/usb_types_ch9.h"
#if !CONFIG_IDF_TARGET_LINUX
#include "esp_private/esp_cache_private.h"
#define ALIGN_UP(num, align) ((align) == 0 ? (num) : (((num) + ((align) - 1)) & ~((align) - 1)))
#endif
#define DATA_BUFFER_CAPS (MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED)
urb_t *urb_alloc(size_t data_buffer_size, int num_isoc_packets)
{
urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (sizeof(usb_isoc_packet_desc_t) * num_isoc_packets), MALLOC_CAP_DEFAULT);
void *data_buffer = heap_caps_malloc(data_buffer_size, MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED);
#if !CONFIG_IDF_TARGET_LINUX
// Note for developers: We do not use heap_caps_get_allocated_size() because it is broken with HEAP_POISONING=COMPREHENSIVE
size_t cache_align = 0;
esp_cache_get_alignment(DATA_BUFFER_CAPS, &cache_align);
data_buffer_size = ALIGN_UP(data_buffer_size, cache_align);
#endif
void *data_buffer = heap_caps_malloc(data_buffer_size, DATA_BUFFER_CAPS);
if (urb == NULL || data_buffer == NULL) {
goto err;
}
// Cast as dummy transfer so that we can assign to const fields
usb_transfer_dummy_t *dummy_transfer = (usb_transfer_dummy_t *)&urb->transfer;
dummy_transfer->data_buffer = data_buffer;
dummy_transfer->data_buffer_size = heap_caps_get_allocated_size(data_buffer);
dummy_transfer->data_buffer_size = data_buffer_size;
dummy_transfer->num_isoc_packets = num_isoc_packets;
return urb;
err: