fix(uhci): rx fsm race condition and buffer size check

Closes https://github.com/espressif/esp-idf/issues/18819 Closes https://github.com/espressif/esp-idf/issues/18820
2026-10-01 18:50:34 +03:00 · 2026-07-29 19:31:30 +08:00
parent a74daf64c8
commit c3a62f97b3
7 changed files with 178 additions and 109 deletions
@@ -21,7 +21,7 @@ typedef struct {
    size_t tx_trans_queue_depth;                          /*!< Depth of internal transfer queue, increase this value can support more transfers pending in the background */
    size_t max_transmit_size;                             /*!< Maximum transfer size in one transaction, in bytes. Note that this is the total size of all buffers combined */
    size_t max_transmit_buffer_count;                     /*!< Maximum number of buffers that can be transmitted together in one transaction, via `uhci_multi_buffer_transmit()`. Set to 0 or 1 if only single-buffer transmit (`uhci_transmit()`) is needed. */
-    size_t max_receive_internal_mem;                      /*!< Internal DMA usage memory. Each DMA node can point to a maximum of x bytes (depends on chip). This value determines the number of DMA nodes used for each transaction. When your transfer size is large enough, it is recommended to set this value greater than x to facilitate efficient ping-pong operations, such as 2 * x. */
+    size_t max_receive_internal_mem;                      /*!< Expected maximum buffer size for uhci_receive(). This value determines the number of descriptors in the receive DMA chain. Each DMA descriptor can reference a buffer of up to X bytes (depending on the chip). For large transfers, at least two descriptors are recommended for ping-pong operation. */
    size_t dma_burst_size;                                /*!< DMA burst size, in bytes. Set to 0 to disable data burst. Otherwise, use a power of 2. */
    size_t max_packet_receive;                            /*!< Max receive size, auto stop receiving after reach this value, only valid when `length_eof` set true */

@@ -79,15 +79,16 @@ esp_err_t uhci_new_controller(const uhci_controller_config_t *config, uhci_contr
 *                            `uhci_new_controller()`.
 * @param[out] read_buffer    Pointer to the buffer where the received data will be stored.
 *                            The buffer must be pre-allocated by the caller.
- * @param[in] buffer_size     The size of read buffer.
+ * @param[in] buffer_size     The size of read buffer. Should generally not exceed `uhci_controller_config_t.max_receive_internal_mem`.
 *
 * @note The function is non-blocking, it just mounts the user buffer to the DMA.
 * The return from the function doesn't mean a finished receive. You need to register corresponding
 * callback function to get notification.
 *
 * @return
- * - `ESP_OK`: Data successfully received and written to the buffer.
- * - `ESP_ERR_INVALID_ARG`: Invalid arguments (e.g., null buffer or invalid controller handle).
+ * - `ESP_OK`: The driver is ready for data reception.
+ * - `ESP_ERR_INVALID_STATE`: The controller is not in enable state.
+ * - `ESP_ERR_INVALID_ARG`: Invalid arguments (e.g., invalid controller handle, null buffer, invalid buffer size).
 */
 esp_err_t uhci_receive(uhci_controller_handle_t uhci_ctrl, uint8_t *read_buffer, size_t buffer_size);

@@ -50,6 +50,8 @@ typedef bool (*uhci_tx_done_callback_t)(uhci_controller_handle_t uhci_ctrl, cons

 /**
 * @brief UHCI RX Done Event Data Structure
+ *
+ * @note When an abnormal EOF occurs, `data` will be NULL and `recv_size` will be 0.
 */
 typedef struct {
    const uint8_t *data;           /*!< Pointer to the received data buffer. Data pointed to by this pointer is typically only guaranteed to be readable during the callback. If you need to use it after callback returns, copy it to external buffer first or refer to advanced zero-copy usage. */
@@ -108,79 +108,73 @@ static bool uhci_gdma_rx_callback_done(gdma_channel_handle_t dma_chan, gdma_even
 {
    bool need_yield = false;
    uhci_controller_handle_t uhci_ctrl = (uhci_controller_handle_t) user_data;
-    bool is_buf_from_psram = esp_ptr_external_ram(uhci_ctrl->rx_dir.buffer_pointers[uhci_ctrl->rx_dir.node_index]);
-    size_t cache_line = uhci_ctrl->rx_dir.cache_line;
-    // If the data is not all received, handle it in not normal_eof block. Otherwise, in eof block.
-    if (!event_data->flags.normal_eof) {
-        size_t rx_size = uhci_ctrl->rx_dir.buffer_size_per_desc_node[uhci_ctrl->rx_dir.node_index];
-        uhci_rx_event_data_t evt_data = {
+
+    // Prevent any spurious interrupts after EOF.
+    if (atomic_load(&uhci_ctrl->rx_dir.rx_fsm) != UHCI_RX_FSM_RUN) {
+        return false;
+    }
+
+    if (event_data->flags.abnormal_eof || event_data->flags.normal_eof) {
+        // An EOF signal does not automatically stop the DMA transfer, so we need to stop it manually.
+        gdma_stop(uhci_ctrl->rx_dir.dma_chan);
+        // stop() cannot prevent already prefetched DMA descriptors from being processed.
+        // A reset() is required to fully halt the DMA engine and eliminate any subsequent spurious interrupts.
+        gdma_reset(uhci_ctrl->rx_dir.dma_chan);
+    }
+
+    uhci_rx_event_data_t evt_data = {0};
+
+    if (!event_data->flags.abnormal_eof) {
+        const size_t cache_line = uhci_ctrl->rx_dir.cache_line;
+
+        size_t rx_size, sync_size;
+        if (!event_data->flags.normal_eof) {
+            rx_size = uhci_ctrl->rx_dir.buffer_size_per_desc_node[uhci_ctrl->rx_dir.node_index];
+            sync_size = rx_size;
+        } else {
+            rx_size = gdma_link_count_buffer_size_till_eof(uhci_ctrl->rx_dir.dma_link, uhci_ctrl->rx_dir.node_index);
+            sync_size = UHCI_ALIGN_UP(rx_size, cache_line);  // round up to the next cache line
+        }
+
+        evt_data = (uhci_rx_event_data_t) {
            .data = uhci_ctrl->rx_dir.buffer_pointers[uhci_ctrl->rx_dir.node_index],
            .recv_size = rx_size,
-            .flags.totally_received = false,
+            .flags.totally_received = event_data->flags.normal_eof,
        };

-        if (is_buf_from_psram) {
+        if (esp_ptr_external_ram(evt_data.data)) {
            esp_psram_mspi_mb();
        }
+
        // DMA just finished writing the node's buffer. Because the descriptor link is circular,
        // the same buffer region gets overwritten on every loop. On targets where the buffer is
-        // backed by a cache, the cache is not snooped by DMA, so the CPU must invalidate the range
-        // before reading, otherwise it will return stale data from a previous loop.
+        // backed by a cache, the CPU must invalidate the range before reading, otherwise it will
+        // return stale data from a previous loop.
        if (cache_line > 0) {
-            // The per-node buffer base is aligned to cache_line (see uhci_receive), and rx_size here
-            // equals buffer_size_per_desc_node[] which is also a multiple of cache_line.
-            esp_cache_msync((void *)evt_data.data, rx_size, ESP_CACHE_MSYNC_FLAG_DIR_M2C);
-        }
-        if (uhci_ctrl->rx_dir.on_rx_trans_event) {
-            need_yield |= uhci_ctrl->rx_dir.on_rx_trans_event(uhci_ctrl, &evt_data, uhci_ctrl->user_data);
-        }
-        uhci_ctrl->rx_dir.node_index++;
-        // Go back to 0 as its a circle descriptor link
-        if (uhci_ctrl->rx_dir.node_index >= uhci_ctrl->rx_dir.rx_num_dma_nodes) {
-            uhci_ctrl->rx_dir.node_index = 0;
+            esp_cache_msync((void *)evt_data.data, sync_size, ESP_CACHE_MSYNC_FLAG_DIR_M2C);
        }
+    }

-    } else {
-        // eof event
-        size_t rx_size = gdma_link_count_buffer_size_till_eof(uhci_ctrl->rx_dir.dma_link, uhci_ctrl->rx_dir.node_index);
-        uhci_rx_event_data_t evt_data = {
-            .data = uhci_ctrl->rx_dir.buffer_pointers[uhci_ctrl->rx_dir.node_index],
-            .recv_size = rx_size,
-            .flags.totally_received = true,
-        };
+    if (event_data->flags.abnormal_eof || event_data->flags.normal_eof) {
+        uhci_ctrl->rx_dir.node_index = 0;

-        if (is_buf_from_psram) {
-            esp_psram_mspi_mb();
-        }
 #if CONFIG_PM_ENABLE
        // release power manager lock
        if (uhci_ctrl->pm_lock) {
            esp_pm_lock_release(uhci_ctrl->pm_lock);
        }
 #endif
-        // Same reasoning as the partial branch. rx_size here may not be a multiple of cache_line
-        // because transfer can end mid-buffer on a UART idle EOF, so round up to the next cache
-        // line (esp_cache_msync's M2C direction requires aligned size and doesn't accept the
-        // UNALIGNED flag). The extra bytes still belong to the user buffer so invalidating them
-        // is harmless.
-        if (cache_line > 0) {
-            size_t sync_size = (rx_size + cache_line - 1) & ~(cache_line - 1);
-            esp_cache_msync((void *)evt_data.data, sync_size, ESP_CACHE_MSYNC_FLAG_DIR_M2C);
-        }
-        if (uhci_ctrl->rx_dir.on_rx_trans_event) {
-            need_yield |= uhci_ctrl->rx_dir.on_rx_trans_event(uhci_ctrl, &evt_data, uhci_ctrl->user_data);
-        }
-
-        // Stop the transaction when EOF is detected. In case for length EOF, there is no further more callback to be invoked.
-        gdma_stop(uhci_ctrl->rx_dir.dma_chan);
-        gdma_reset(uhci_ctrl->rx_dir.dma_chan);
-
        atomic_store(&uhci_ctrl->rx_dir.rx_fsm, UHCI_RX_FSM_ENABLE);
-        uhci_ctrl->rx_dir.node_index = 0;
+    } else {
+        uhci_ctrl->rx_dir.node_index++;
+        // Go back to 0 as its a circle descriptor link
+        if (uhci_ctrl->rx_dir.node_index >= uhci_ctrl->rx_dir.rx_num_dma_nodes) {
+            uhci_ctrl->rx_dir.node_index = 0;
+        }
    }

-    if (event_data->flags.abnormal_eof) {
-        esp_rom_printf(DRAM_STR("An abnormal eof on uhci detected\n"));
+    if (uhci_ctrl->rx_dir.on_rx_trans_event) {
+        need_yield |= uhci_ctrl->rx_dir.on_rx_trans_event(uhci_ctrl, &evt_data, uhci_ctrl->user_data);
    }

    return need_yield;
@@ -318,59 +312,73 @@ static void uhci_do_transmit(uhci_controller_handle_t uhci_ctrl, uhci_transactio
 esp_err_t uhci_receive(uhci_controller_handle_t uhci_ctrl, uint8_t *read_buffer, size_t buffer_size)
 {
    ESP_RETURN_ON_FALSE(uhci_ctrl, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
-    ESP_RETURN_ON_FALSE((read_buffer != NULL), ESP_ERR_INVALID_ARG, TAG, "read buffer null");
+    ESP_RETURN_ON_FALSE(read_buffer != NULL && buffer_size > 0, ESP_ERR_INVALID_ARG, TAG, "read buffer null or buffer size is 0");

-    uint32_t mem_cache_line_size = esp_ptr_external_ram(read_buffer) ? uhci_ctrl->ext_mem_cache_line_size : uhci_ctrl->int_mem_cache_line_size;
+    uhci_rx_fsm_t expected_fsm = UHCI_RX_FSM_ENABLE;
+    ESP_RETURN_ON_FALSE(atomic_compare_exchange_strong(&uhci_ctrl->rx_dir.rx_fsm, &expected_fsm, UHCI_RX_FSM_RUN_WAIT), ESP_ERR_INVALID_STATE, TAG, "controller not in enable state");

+    esp_err_t ret = ESP_OK;
+
+    const uint32_t mem_cache_line_size = esp_ptr_external_ram(read_buffer) ? uhci_ctrl->ext_mem_cache_line_size : uhci_ctrl->int_mem_cache_line_size;
    // Must take cache line into consideration for C2M operation.
-    uint32_t max_alignment_needed = UHCI_MAX(UHCI_MAX(uhci_ctrl->rx_dir.int_mem_align, uhci_ctrl->rx_dir.ext_mem_align), mem_cache_line_size);
+    const uint32_t max_alignment_needed = UHCI_MAX(UHCI_MAX(uhci_ctrl->rx_dir.int_mem_align, uhci_ctrl->rx_dir.ext_mem_align), mem_cache_line_size);
+    uhci_ctrl->rx_dir.cache_line = mem_cache_line_size;

    // Align the read_buffer pointer to mem_cache_line_size
    if (max_alignment_needed > 0 && (((uintptr_t)read_buffer) & (max_alignment_needed - 1)) != 0) {
        uintptr_t aligned_address = ((uintptr_t)read_buffer + max_alignment_needed - 1) & ~(max_alignment_needed - 1);
        size_t offset = aligned_address - (uintptr_t)read_buffer;

-        ESP_RETURN_ON_FALSE(buffer_size > offset, ESP_ERR_INVALID_ARG, TAG, "buffer size too small to align");
+        ESP_GOTO_ON_FALSE(buffer_size > offset, ESP_ERR_INVALID_ARG, err, TAG, "buffer size too small to align");

        read_buffer = (uint8_t *)aligned_address;
        buffer_size -= offset;
    }

-    uhci_ctrl->rx_dir.cache_line = mem_cache_line_size;
-    uhci_rx_fsm_t expected_fsm = UHCI_RX_FSM_ENABLE;
-    ESP_RETURN_ON_FALSE(atomic_compare_exchange_strong(&uhci_ctrl->rx_dir.rx_fsm, &expected_fsm, UHCI_RX_FSM_RUN_WAIT), ESP_ERR_INVALID_STATE, TAG, "controller not in enable state");
-
-    size_t node_count = uhci_ctrl->rx_dir.rx_num_dma_nodes;
-
+    const size_t node_count = uhci_ctrl->rx_dir.rx_num_dma_nodes;
    // Initialize the mount configurations for each DMA node, making sure every node is properly aligned.
    size_t usable_size = (max_alignment_needed == 0) ? buffer_size : (buffer_size / max_alignment_needed) * max_alignment_needed;
    size_t base_size = (max_alignment_needed == 0) ? usable_size / node_count : (usable_size / node_count / max_alignment_needed) * max_alignment_needed;
    size_t remaining_size = usable_size - (base_size * node_count);

-    gdma_buffer_mount_config_t mount_configs[node_count];
-    memset(mount_configs, 0, node_count * sizeof(gdma_buffer_mount_config_t));
+    {
+        gdma_buffer_mount_config_t mount_configs[node_count];
+        memset(mount_configs, 0, node_count * sizeof(gdma_buffer_mount_config_t));

-    for (size_t i = 0; i < node_count; i++) {
-        uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] = base_size;
-        uhci_ctrl->rx_dir.buffer_pointers[i] = read_buffer;
-        size_t buffer_alignment = esp_ptr_internal(read_buffer) ? uhci_ctrl->rx_dir.int_mem_align : uhci_ctrl->rx_dir.ext_mem_align;
-        // Distribute the remaining size to the first few nodes
-        if (remaining_size >= max_alignment_needed) {
-            uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] += max_alignment_needed;
-            remaining_size -= max_alignment_needed;
+        for (size_t i = 0; i < node_count; i++) {
+            uhci_ctrl->rx_dir.buffer_pointers[i] = read_buffer;
+            // Distribute the remaining size to the first few nodes
+            if (remaining_size >= max_alignment_needed) {
+                uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] = base_size + max_alignment_needed;
+                remaining_size -= max_alignment_needed;
+            } else {
+                uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] = base_size;
+            }
+            ESP_GOTO_ON_FALSE(uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] != 0 && uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] <= DMA_DESCRIPTOR_BUFFER_MAX_SIZE,
+                              ESP_ERR_INVALID_ARG, err, TAG, "buffer_size is too small or too large");
+
+            size_t buffer_alignment = esp_ptr_internal(read_buffer) ? uhci_ctrl->rx_dir.int_mem_align : uhci_ctrl->rx_dir.ext_mem_align;
+            mount_configs[i] = (gdma_buffer_mount_config_t) {
+                .buffer = read_buffer,
+                .buffer_alignment = buffer_alignment,
+                .length = uhci_ctrl->rx_dir.buffer_size_per_desc_node[i],
+                .flags = {
+                    .mark_final = GDMA_FINAL_LINK_TO_DEFAULT,
+                }
+            };
+            ESP_LOGD(TAG, "The DMA node %d has %d byte", i, uhci_ctrl->rx_dir.buffer_size_per_desc_node[i]);
+
+            read_buffer += uhci_ctrl->rx_dir.buffer_size_per_desc_node[i];
        }

-        mount_configs[i] = (gdma_buffer_mount_config_t) {
-            .buffer = read_buffer,
-            .buffer_alignment = buffer_alignment,
-            .length = uhci_ctrl->rx_dir.buffer_size_per_desc_node[i],
-            .flags = {
-                .mark_final = GDMA_FINAL_LINK_TO_DEFAULT,
-            }
-        };
-        ESP_LOGD(TAG, "The DMA node %d has %d byte", i, uhci_ctrl->rx_dir.buffer_size_per_desc_node[i]);
-        ESP_RETURN_ON_FALSE(uhci_ctrl->rx_dir.buffer_size_per_desc_node[i] != 0, ESP_ERR_INVALID_STATE, TAG, "Allocate dma node length is 0, please reconfigure the buffer_size");
-        read_buffer += uhci_ctrl->rx_dir.buffer_size_per_desc_node[i];
+        ESP_GOTO_ON_ERROR(gdma_link_mount_buffers(uhci_ctrl->rx_dir.dma_link, 0, mount_configs, node_count, NULL), err, TAG, "DMA link mount buffers failed");
+
+        // Invalidate cache before DMA starts to ensure no dirty cache lines.
+        // All DMA nodes (mount_configs) share the same contiguous user buffer, so checking mount_configs[0].buffer is sufficient.
+        bool need_cache_sync = esp_ptr_internal(mount_configs[0].buffer) ? (uhci_ctrl->int_mem_cache_line_size > 0) : (uhci_ctrl->ext_mem_cache_line_size > 0);
+        if (need_cache_sync) {
+            ESP_GOTO_ON_ERROR(esp_cache_msync(mount_configs[0].buffer, usable_size, ESP_CACHE_MSYNC_FLAG_DIR_M2C), err, TAG, "cache sync failed");
+        }
    }

 #if CONFIG_PM_ENABLE
@@ -380,21 +388,15 @@ esp_err_t uhci_receive(uhci_controller_handle_t uhci_ctrl, uint8_t *read_buffer,
    }
 #endif

-    gdma_link_mount_buffers(uhci_ctrl->rx_dir.dma_link, 0, mount_configs, node_count, NULL);
-
-    // Invalidate cache before DMA starts to ensure no dirty cache lines.
-    // All DMA nodes (mount_configs) share the same contiguous user buffer, so checking mount_configs[0].buffer is sufficient.
-    bool need_cache_sync = esp_ptr_internal(mount_configs[0].buffer) ? (uhci_ctrl->int_mem_cache_line_size > 0) : (uhci_ctrl->ext_mem_cache_line_size > 0);
-    if (need_cache_sync) {
-        ESP_RETURN_ON_ERROR(esp_cache_msync(mount_configs[0].buffer, usable_size, ESP_CACHE_MSYNC_FLAG_DIR_M2C), TAG, "cache sync failed");
-    }
-
    atomic_store(&uhci_ctrl->rx_dir.rx_fsm, UHCI_RX_FSM_RUN);

    gdma_reset(uhci_ctrl->rx_dir.dma_chan);
    gdma_start(uhci_ctrl->rx_dir.dma_chan, gdma_link_get_head_addr(uhci_ctrl->rx_dir.dma_link));

    return ESP_OK;
+err:
+    atomic_store(&uhci_ctrl->rx_dir.rx_fsm, UHCI_RX_FSM_ENABLE);
+    return ret;
 }

 esp_err_t uhci_multi_buffer_transmit(uhci_controller_handle_t uhci_ctrl, const uhci_transmit_buffer_info_t *buffer_info_array, size_t array_size)
@@ -478,20 +480,29 @@ esp_err_t uhci_del_controller(uhci_controller_handle_t uhci_ctrl)
 {
    ESP_RETURN_ON_FALSE(uhci_ctrl, ESP_ERR_INVALID_ARG, TAG, "invalid argument");

-    if (uhci_ctrl->rx_dir.rx_fsm != UHCI_RX_FSM_ENABLE) {
+    uhci_rx_fsm_t expected_rx = UHCI_RX_FSM_ENABLE;
+    if (!atomic_compare_exchange_strong(&uhci_ctrl->rx_dir.rx_fsm, &expected_rx, UHCI_RX_FSM_DELETE)) {
        ESP_LOGE(TAG, "RX transaction is not finished, delete controller failed");
        return ESP_ERR_INVALID_STATE;
    }

-    if (uhci_ctrl->tx_dir.tx_fsm != UHCI_TX_FSM_ENABLE) {
+    uhci_tx_fsm_t expected_tx = UHCI_TX_FSM_ENABLE;
+    if (!atomic_compare_exchange_strong(&uhci_ctrl->tx_dir.tx_fsm, &expected_tx, UHCI_TX_FSM_DELETE)) {
        ESP_LOGE(TAG, "TX transaction is not finished, delete controller failed");
+        atomic_store(&uhci_ctrl->rx_dir.rx_fsm, UHCI_RX_FSM_ENABLE);  // rollback
        return ESP_ERR_INVALID_STATE;
    }

+    // Ensure that all interrupts (GDMA callbacks) have completed and that no further callbacks can be
+    // triggered before releasing the resources.
+    ESP_RETURN_ON_ERROR(uhci_gdma_deinitialize(uhci_ctrl), TAG, "deinitialize uhci dma channel failed");
+
    PERIPH_RCC_ATOMIC() {
        uhci_ll_enable_bus_clock(uhci_ctrl->uhci_num, false);
    }

+    uhci_hal_deinit(&uhci_ctrl->hal);
+
    for (int i = 0; i < UHCI_TRANS_QUEUE_MAX; i++) {
        if (uhci_ctrl->tx_dir.trans_queues[i]) {
            vQueueDeleteWithCaps(uhci_ctrl->tx_dir.trans_queues[i]);
@@ -519,10 +530,6 @@ esp_err_t uhci_del_controller(uhci_controller_handle_t uhci_ctrl)
    }
 #endif

-    ESP_RETURN_ON_ERROR(uhci_gdma_deinitialize(uhci_ctrl), TAG, "deinitialize uhci dam channel failed");
-
-    uhci_hal_deinit(&uhci_ctrl->hal);
-
    s_uhci_platform.controller[uhci_ctrl->uhci_num] = NULL;

    heap_caps_free(uhci_ctrl);
@@ -44,6 +44,7 @@ typedef enum {
    UHCI_TX_FSM_ENABLE,      /**< FSM is enabling the UHCI system. */
    UHCI_TX_FSM_RUN_WAIT,    /**< FSM is waiting to transition to the running state. */
    UHCI_TX_FSM_RUN,         /**< FSM is in the running state, actively handling UHCI operations. */
+    UHCI_TX_FSM_DELETE,      /**< FSM is claimed by uhci_del_controller() for teardown, no new transaction is accepted. */
 } uhci_tx_fsm_t;

 typedef enum {
@@ -58,6 +59,7 @@ typedef enum {
    UHCI_RX_FSM_ENABLE,      /**< FSM is enabling the UHCI system. */
    UHCI_RX_FSM_RUN_WAIT,    /**< FSM is waiting to transition to the running state. */
    UHCI_RX_FSM_RUN,         /**< FSM is in the running state, actively handling UHCI operations. */
+    UHCI_RX_FSM_DELETE,      /**< FSM is claimed by uhci_del_controller() for teardown, no new transaction is accepted. */
 } uhci_rx_fsm_t;

 typedef struct {
@@ -74,6 +74,63 @@ TEST_CASE("UHCI controller install-uninstall test", "[uhci]")
    TEST_ESP_OK(uhci_del_controller(uhci_ctrl));
 }

+TEST_CASE("UHCI receive/transmit reject invalid buffer size", "[uhci]")
+{
+    uhci_controller_config_t uhci_cfg = {
+        .uart_port = EX_UART_NUM,
+        .tx_trans_queue_depth = 3,
+        .max_receive_internal_mem = 2 * 1024,
+        .max_transmit_size = 2 * 1024,
+        .max_transmit_buffer_count = 2,
+        .dma_burst_size = 32,
+        .rx_eof_flags.idle_eof = 1,
+    };
+
+    uhci_controller_handle_t uhci_ctrl;
+    TEST_ESP_OK(uhci_new_controller(&uhci_cfg, &uhci_ctrl));
+
+    const size_t oversize = 5 * 1024;
+    uint8_t *big_buf = heap_caps_calloc(1, oversize, MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+    TEST_ASSERT_NOT_NULL(big_buf);
+
+    // -------- uhci_receive --------
+
+    // buffer_size == 0 must be rejected.
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_receive(uhci_ctrl, big_buf, 0));
+
+    // Excessively large buffers must be rejected.
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_receive(uhci_ctrl, big_buf, oversize));
+
+    // -------- uhci_transmit / uhci_multi_buffer_transmit --------
+
+    // write_size == 0 must be rejected.
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_transmit(uhci_ctrl, big_buf, 0));
+
+    // A single write_size exceeding max_transmit_size must be rejected.
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_transmit(uhci_ctrl, big_buf, oversize));
+
+    // Same limit applies to the combined size of uhci_multi_buffer_transmit segments, even
+    // though each individual segment is within max_transmit_size.
+    uhci_transmit_buffer_info_t buf_info[2] = {
+        { .write_buffer = big_buf, .buffer_size = uhci_cfg.max_transmit_size },
+        { .write_buffer = big_buf, .buffer_size = uhci_cfg.max_transmit_size },
+    };
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_multi_buffer_transmit(uhci_ctrl, buf_info, 2));
+
+    // A NULL or zero-size segment inside the array must also be rejected.
+    buf_info[1].write_buffer = NULL;
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_multi_buffer_transmit(uhci_ctrl, buf_info, 2));
+    buf_info[1].write_buffer = big_buf;
+    buf_info[1].buffer_size = 0;
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG, uhci_multi_buffer_transmit(uhci_ctrl, buf_info, 2));
+
+    free(big_buf);
+
+    // The controller must still be in a clean, deletable state after all the rejected
+    // calls above, which confirms the RX/TX FSMs were correctly rolled back on error.
+    TEST_ESP_OK(uhci_del_controller(uhci_ctrl));
+}
+
 typedef enum {
    UHCI_EVT_PARTIAL_DATA,
    UHCI_EVT_EOF,
@@ -149,7 +206,7 @@ static void uhci_receive_test(void *arg)
                if (evt == UHCI_EVT_EOF) {
                    disp_buf(receive_data, ctx->receive_size);
                    for (int i = 0; i < ctx->receive_size; i++) {
-                        TEST_ASSERT(receive_data[i] == (uint8_t)i);
+                        TEST_ASSERT_EQUAL(receive_data[i], (uint8_t)i);
                    }
                    printf("Received size: %d\n", ctx->receive_size);
                    break;