From a68dbf40338b183d50fc0d74ad277f421a4a2391 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 9 Jul 2026 18:11:49 +0800 Subject: [PATCH 1/3] fix(bitscrambler): clean up loopback create failures Clear the returned handle on loopback creation failure and route initialized objects through bitscrambler_free so channel ownership and extra cleanup state cannot leak after partial setup errors. Add a regression test for the failed create path. --- .../src/bitscrambler_loopback.c | 14 +++++++++----- .../bitscrambler/main/test_bitscrambler.c | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/components/esp_driver_bitscrambler/src/bitscrambler_loopback.c b/components/esp_driver_bitscrambler/src/bitscrambler_loopback.c index 7cf0c7b1ca1..8b9d78fc3f4 100644 --- a/components/esp_driver_bitscrambler/src/bitscrambler_loopback.c +++ b/components/esp_driver_bitscrambler/src/bitscrambler_loopback.c @@ -72,11 +72,13 @@ esp_err_t bitscrambler_loopback_create(bitscrambler_handle_t *handle, int attach if (!handle) { return ESP_ERR_INVALID_ARG; } + *handle = NULL; if (attach_to < 0 || attach_to > SOC_BITSCRAMBLER_ATTACH_MAX) { return ESP_ERR_INVALID_ARG; } esp_err_t ret = ESP_OK; + bool bs_initialized = false; bitscrambler_loopback_t *bs = calloc(1, sizeof(bitscrambler_loopback_t)); if (!bs) { return ESP_ERR_NO_MEM; @@ -88,14 +90,13 @@ esp_err_t bitscrambler_loopback_create(bitscrambler_handle_t *handle, int attach .attach_to = attach_to }; ESP_GOTO_ON_ERROR(bitscrambler_init_loopback(&bs->bs, &cfg), err, TAG, "failed bitscrambler init for loopback"); + bs_initialized = true; // register extra cleanup function to free loopback resources bitscrambler_register_extra_clean_up(&bs->bs, bitscrambler_loopback_cleanup, bs); bs->sema_done = xSemaphoreCreateBinary(); - if (!bs->sema_done) { - goto err; - } + ESP_GOTO_ON_FALSE(bs->sema_done, ESP_ERR_NO_MEM, err, TAG, "failed to create semaphore"); bs->max_transfer_sz_bytes = max_transfer_sz_bytes; size_t desc_ct = esp_dma_calculate_node_count(max_transfer_sz_bytes, 4, DMA_DESCRIPTOR_BUFFER_MAX_SIZE); @@ -137,8 +138,11 @@ esp_err_t bitscrambler_loopback_create(bitscrambler_handle_t *handle, int attach return ESP_OK; err: - bitscrambler_loopback_free(bs); - free(bs); + if (bs_initialized) { + bitscrambler_free(&bs->bs); + } else { + free(bs); + } return ret; } diff --git a/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c b/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c index b4cbc383ec3..655a38b7671 100644 --- a/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c +++ b/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ #include +#include #include "sdkconfig.h" #include "unity.h" #include "unity_test_utils.h" @@ -59,6 +60,19 @@ TEST_CASE("Timeout on stuck program", "[bs]") free(data_out); } +TEST_CASE("Loopback create failure clears handle and releases channels", "[bs]") +{ + bitscrambler_handle_t failed_bs = (bitscrambler_handle_t)0x1; + + esp_err_t err = bitscrambler_loopback_create(&failed_bs, SOC_BITSCRAMBLER_ATTACH_GPSPI2, SIZE_MAX / 2); + TEST_ASSERT_NOT_EQUAL(ESP_OK, err); + TEST_ASSERT_NULL(failed_bs); + + bitscrambler_handle_t bs = NULL; + TEST_ESP_OK(bitscrambler_loopback_create(&bs, SOC_BITSCRAMBLER_ATTACH_GPSPI2, 4096)); + bitscrambler_free(bs); +} + TEST_CASE("BitScrambler with EOF counted on upstream", "[bs]") { const size_t len = 32; From c284036a366ca6bc1680db8d56665c2be65f341b Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 9 Jul 2026 18:54:55 +0800 Subject: [PATCH 2/3] fix(bitscrambler): validate and safely load LUT data Reject LUT loads that exceed the hardware address space and assemble each 32-bit LUT word via memcpy so unaligned and partial inputs do not read past the caller buffer. Add regression tests for oversized and partial unaligned LUT loads. --- .../include/driver/bitscrambler.h | 1 + .../src/bitscrambler.c | 22 ++++++++- .../bitscrambler/main/test_bitscrambler.c | 46 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/components/esp_driver_bitscrambler/include/driver/bitscrambler.h b/components/esp_driver_bitscrambler/include/driver/bitscrambler.h index 47e7099c957..296c7ea91a3 100644 --- a/components/esp_driver_bitscrambler/include/driver/bitscrambler.h +++ b/components/esp_driver_bitscrambler/include/driver/bitscrambler.h @@ -89,6 +89,7 @@ esp_err_t bitscrambler_load_program(bitscrambler_handle_t handle, const void *pr * @return * - ESP_OK * - ESP_ERR_INVALID_ARG: Invalid handle or lut pointer + * - ESP_ERR_INVALID_SIZE: LUT data exceeds hardware capacity */ esp_err_t bitscrambler_load_lut(bitscrambler_handle_t handle, void *lut, size_t size_bytes); diff --git a/components/esp_driver_bitscrambler/src/bitscrambler.c b/components/esp_driver_bitscrambler/src/bitscrambler.c index 2a3cfb49319..9a9917148be 100644 --- a/components/esp_driver_bitscrambler/src/bitscrambler.c +++ b/components/esp_driver_bitscrambler/src/bitscrambler.c @@ -23,6 +23,8 @@ static const char *TAG = "bitscrambler"; #define BITSCRAMBLER_BINARY_VER 1 //max version we're compatible with #define BITSCRAMBLER_HW_REV 0 +// LUT index register is 11 bits wide, so the LUT address space is 2048 bytes. +#define BITSCRAMBLER_LUT_MAX_BYTES (1U << 11) // After a reset, it can take a few cycles for the BitScrambler to actually be // reset. We check this many times for this; if it takes longer the hardware @@ -241,12 +243,28 @@ esp_err_t bitscrambler_load_lut(bitscrambler_handle_t handle, void *lut, size_t if (!handle || !lut) { return ESP_ERR_INVALID_ARG; } - uint32_t *lut_words = (uint32_t*)lut; + if (size_bytes > BITSCRAMBLER_LUT_MAX_BYTES) { + return ESP_ERR_INVALID_SIZE; + } + const uint8_t *lut_bytes = (const uint8_t *)lut; bitscrambler_lut_width_t lut_width = bitscrambler_ll_get_lut_width(handle->hw, handle->cfg.dir); bitscrambler_ll_set_lut_width(handle->hw, handle->cfg.dir, BITSCRAMBLER_LUT_WIDTH_32BIT); size_t size_words = (size_bytes + 3) / 4; for (int w = 0; w < size_words; w++) { - bitscrambler_ll_lutmem_write(handle->hw, handle->cfg.dir, w, lut_words[w]); + // Assemble each LUT entry byte-wise before writing it to hardware: + // 1) callers are allowed to pass unaligned buffers, so reading via a + // uint32_t * could fault or perform an unaligned access on some targets; + // 2) the final LUT word may be only partially supplied, and copying the + // valid bytes into a zero-initialized word avoids reading past the end + // of the caller's buffer and leaking adjacent memory into the LUT. + uint32_t lut_word = 0; + size_t offset = w * sizeof(lut_word); + size_t bytes_to_copy = size_bytes - offset; + if (bytes_to_copy > sizeof(lut_word)) { + bytes_to_copy = sizeof(lut_word); + } + memcpy(&lut_word, lut_bytes + offset, bytes_to_copy); + bitscrambler_ll_lutmem_write(handle->hw, handle->cfg.dir, w, lut_word); } bitscrambler_ll_set_lut_width(handle->hw, handle->cfg.dir, lut_width); return ESP_OK; diff --git a/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c b/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c index 655a38b7671..dd6a28a8823 100644 --- a/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c +++ b/components/esp_driver_bitscrambler/test_apps/bitscrambler/main/test_bitscrambler.c @@ -5,6 +5,7 @@ */ #include #include +#include #include "sdkconfig.h" #include "unity.h" #include "unity_test_utils.h" @@ -171,6 +172,51 @@ TEST_CASE("BitScrambler with LUT32", "[bs]") free(data_out); } +TEST_CASE("BitScrambler loads partial LUT from unaligned buffer", "[bs]") +{ + const size_t len = 32; + const uint32_t expected_lut_words[] = { + 0xA0011111, + 0xA0022222, + 0xA0033333, + 0x00000044, + }; + const uint32_t lut_source_words[] = { + 0xA0011111, + 0xA0022222, + 0xA0033333, + 0xA0044444, + }; + uint8_t lut_bytes[sizeof(lut_source_words) + 4] = {0}; + uint8_t *unaligned_lut = lut_bytes + 1; + const size_t lut_size = 13; + uint8_t *data_in = heap_caps_aligned_calloc(8, 1, len, MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); + uint32_t *data_out = heap_caps_aligned_calloc(8, 1, len * 4, MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); + TEST_ASSERT_NOT_NULL(data_in); + TEST_ASSERT_NOT_NULL(data_out); + memcpy(unaligned_lut, lut_source_words, sizeof(lut_source_words)); + unaligned_lut[lut_size] = 0xAB; + unaligned_lut[lut_size + 1] = 0xCD; + unaligned_lut[lut_size + 2] = 0xEF; + + bitscrambler_handle_t bs; + TEST_ESP_OK(bitscrambler_loopback_create(&bs, SOC_BITSCRAMBLER_ATTACH_GPSPI2, len * 4)); + TEST_ESP_OK(bitscrambler_load_program(bs, bitscrambler_program_lut32)); + TEST_ESP_OK(bitscrambler_load_lut(bs, unaligned_lut, lut_size)); + + size_t res_len = 0; + TEST_ESP_OK(bitscrambler_loopback_run(bs, data_in, len, data_out, len * 4, &res_len)); + bitscrambler_free(bs); + + for (size_t i = 0; i < res_len / 4; i++) { + TEST_ASSERT_EQUAL(expected_lut_words[i % 4], data_out[i]); + } + TEST_ASSERT_EQUAL(len * 4, res_len); + + free(data_in); + free(data_out); +} + TEST_CASE("BitScrambler with loop instruction", "[bs]") { uint8_t data_in[] = {0x00, 0xFF, 0x55}; From c269e6d35ecc873bd05855ff9ce312dfb0a283d6 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 9 Jul 2026 19:16:51 +0800 Subject: [PATCH 3/3] fix(bitscrambler): reject malformed program headers Validate BitScrambler program headers against the supported format and hardware limits before using header-derived instruction and LUT sizes. Also add regression coverage for malformed headers and document that the program blob must come from a trusted assembler output. --- .../include/driver/bitscrambler.h | 5 +++ .../src/bitscrambler.c | 34 +++++++++++++++---- .../esp32c5/include/hal/bitscrambler_ll.h | 3 ++ .../esp32p4/include/hal/bitscrambler_ll.h | 3 ++ .../esp32s31/include/hal/bitscrambler_ll.h | 3 ++ 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/components/esp_driver_bitscrambler/include/driver/bitscrambler.h b/components/esp_driver_bitscrambler/include/driver/bitscrambler.h index 296c7ea91a3..ea9c5f53604 100644 --- a/components/esp_driver_bitscrambler/include/driver/bitscrambler.h +++ b/components/esp_driver_bitscrambler/include/driver/bitscrambler.h @@ -70,6 +70,11 @@ void bitscrambler_free(bitscrambler_handle_t handle); /** * @brief Load a BitScrambler binary program into BitScrambler memory * + * @note The program blob is expected to come from a trusted BitScrambler assembler + * output. This API validates the header fields against hardware limits, but it + * does not take an explicit blob length and therefore cannot verify that an + * arbitrary caller-supplied buffer is complete. + * * @param handle BitScrambler handle * @param program Binary program to load * diff --git a/components/esp_driver_bitscrambler/src/bitscrambler.c b/components/esp_driver_bitscrambler/src/bitscrambler.c index 9a9917148be..6f4ab273e97 100644 --- a/components/esp_driver_bitscrambler/src/bitscrambler.c +++ b/components/esp_driver_bitscrambler/src/bitscrambler.c @@ -23,8 +23,7 @@ static const char *TAG = "bitscrambler"; #define BITSCRAMBLER_BINARY_VER 1 //max version we're compatible with #define BITSCRAMBLER_HW_REV 0 -// LUT index register is 11 bits wide, so the LUT address space is 2048 bytes. -#define BITSCRAMBLER_LUT_MAX_BYTES (1U << 11) +#define BITSCRAMBLER_MAX_LUT_WORDS (BITSCRAMBLER_LL_LUT_MAX_BYTES / sizeof(uint32_t)) // After a reset, it can take a few cycles for the BitScrambler to actually be // reset. We check this many times for this; if it takes longer the hardware @@ -51,6 +50,9 @@ typedef struct { } bitscrambler_program_hdr_t; #define INST_LEN_WORDS BITSCRAMBLER_LL_INST_LEN_WORDS +#define BITSCRAMBLER_V1_HDR_LEN_WORDS (sizeof(bitscrambler_program_hdr_t) / sizeof(uint32_t)) + +_Static_assert(sizeof(bitscrambler_program_hdr_t) % sizeof(uint32_t) == 0, "bitscrambler program header must be word aligned"); // For now, hardware only has one TX and on RX unit. Need to make this more flexible if we get // non-specific and/or more channels. @@ -187,7 +189,7 @@ esp_err_t bitscrambler_load_program(bitscrambler_handle_t bs, const void *progra //Parse the program header. There are two versions, V0 is generated by the C assembler while //v1 is generated by the Python assembler. - int inst_len_bytes = INST_LEN_WORDS * sizeof(uint32_t); //note this is different for v1 and v0 + size_t inst_len_bytes = INST_LEN_WORDS * sizeof(uint32_t); //note this is different for v1 and v0 memcpy(&hdr, program_bin, sizeof(bitscrambler_program_hdr_t)); if (hdr.version != BITSCRAMBLER_BINARY_VER) { ESP_LOGE(TAG, "Bitscrambler binary version %d not supported!", hdr.version); @@ -197,12 +199,29 @@ esp_err_t bitscrambler_load_program(bitscrambler_handle_t bs, const void *progra ESP_LOGE(TAG, "Bitscrambler hardware rev %d not supported!", hdr.hw_rev); return ESP_ERR_INVALID_ARG; } + if (hdr.hdr_len != BITSCRAMBLER_V1_HDR_LEN_WORDS) { + ESP_LOGE(TAG, "Bitscrambler header length %d not supported!", hdr.hdr_len); + return ESP_ERR_INVALID_ARG; + } + if (hdr.inst_ct > BITSCRAMBLER_LL_MAX_INST) { + ESP_LOGE(TAG, "Bitscrambler instruction count %d exceeds hardware limit!", hdr.inst_ct); + return ESP_ERR_INVALID_ARG; + } + if (hdr.lut_word_ct > BITSCRAMBLER_MAX_LUT_WORDS) { + ESP_LOGE(TAG, "Bitscrambler LUT size %d words exceeds hardware limit!", hdr.lut_word_ct); + return ESP_ERR_INVALID_ARG; + } + if (hdr.lut_width > BITSCRAMBLER_LUT_WIDTH_32BIT) { + ESP_LOGE(TAG, "Bitscrambler LUT width %d not supported!", hdr.lut_width); + return ESP_ERR_INVALID_ARG; + } bitscrambler_ll_set_state(bs->hw, bs->cfg.dir, BITSCRAMBLER_SET_STATE_HALT); //Load the program const uint8_t *p = (const uint8_t*)program_bin; - p += hdr.hdr_len * sizeof(uint32_t); //skip header + size_t header_size_bytes = hdr.hdr_len * sizeof(uint32_t); + p += header_size_bytes; //skip header uint32_t instr[INST_LEN_WORDS]; for (int inst = 0; inst < hdr.inst_ct; inst++) { //v0 doesn't have the words 32-bit aligned, so memcpy to work around that @@ -216,9 +235,10 @@ esp_err_t bitscrambler_load_program(bitscrambler_handle_t bs, const void *progra ESP_LOGD(TAG, "Loaded %d instructions", hdr.inst_ct); //Load the LUT. bitscrambler_ll_set_lut_width(bs->hw, bs->cfg.dir, BITSCRAMBLER_LUT_WIDTH_32BIT); - uint32_t *lut = (uint32_t*)p; for (int w = 0; w < hdr.lut_word_ct; w++) { - bitscrambler_ll_lutmem_write(bs->hw, bs->cfg.dir, w, lut[w]); + uint32_t lut_word; + memcpy(&lut_word, p + (w * sizeof(lut_word)), sizeof(lut_word)); + bitscrambler_ll_lutmem_write(bs->hw, bs->cfg.dir, w, lut_word); } //Set options from header @@ -243,7 +263,7 @@ esp_err_t bitscrambler_load_lut(bitscrambler_handle_t handle, void *lut, size_t if (!handle || !lut) { return ESP_ERR_INVALID_ARG; } - if (size_bytes > BITSCRAMBLER_LUT_MAX_BYTES) { + if (size_bytes > BITSCRAMBLER_LL_LUT_MAX_BYTES) { return ESP_ERR_INVALID_SIZE; } const uint8_t *lut_bytes = (const uint8_t *)lut; diff --git a/components/esp_hal_dma/esp32c5/include/hal/bitscrambler_ll.h b/components/esp_hal_dma/esp32c5/include/hal/bitscrambler_ll.h index 838e6f9b342..170ab27d227 100644 --- a/components/esp_hal_dma/esp32c5/include/hal/bitscrambler_ll.h +++ b/components/esp_hal_dma/esp32c5/include/hal/bitscrambler_ll.h @@ -23,6 +23,9 @@ extern "C" { #define BITSCRAMBLER_LL_GET_HW(num) (((num) == 0) ? (&BITSCRAMBLER) : NULL) #define BITSCRAMBLER_LL_INST_LEN_WORDS 9 //length of one instruction in 32-bit words as defined by HW +// LUT index register is 11 bits wide, so the LUT address space is 2048 bytes. +#define BITSCRAMBLER_LL_LUT_MAX_BYTES (1U << 11) +#define BITSCRAMBLER_LL_MAX_INST 8 typedef enum { BITSCRAMBLER_LL_MEM_LP_MODE_SHUT_DOWN, // memory will be powered down during low power stage diff --git a/components/esp_hal_dma/esp32p4/include/hal/bitscrambler_ll.h b/components/esp_hal_dma/esp32p4/include/hal/bitscrambler_ll.h index 14a6c653195..c2d46d7746e 100644 --- a/components/esp_hal_dma/esp32p4/include/hal/bitscrambler_ll.h +++ b/components/esp_hal_dma/esp32p4/include/hal/bitscrambler_ll.h @@ -23,6 +23,9 @@ extern "C" { #define BITSCRAMBLER_LL_GET_HW(num) (((num) == 0) ? (&BITSCRAMBLER) : NULL) #define BITSCRAMBLER_LL_INST_LEN_WORDS 9 //length of one instruction in 32-bit words as defined by HW +// LUT index register is 11 bits wide, so the LUT address space is 2048 bytes. +#define BITSCRAMBLER_LL_LUT_MAX_BYTES (1U << 11) +#define BITSCRAMBLER_LL_MAX_INST 8 typedef enum { BITSCRAMBLER_LL_MEM_LP_MODE_SHUT_DOWN, // memory will be powered down during low power stage diff --git a/components/esp_hal_dma/esp32s31/include/hal/bitscrambler_ll.h b/components/esp_hal_dma/esp32s31/include/hal/bitscrambler_ll.h index 87cbf033fd1..268f39672d3 100644 --- a/components/esp_hal_dma/esp32s31/include/hal/bitscrambler_ll.h +++ b/components/esp_hal_dma/esp32s31/include/hal/bitscrambler_ll.h @@ -22,6 +22,9 @@ extern "C" { #define BITSCRAMBLER_LL_GET_HW(num) (((num) == 0) ? (&BITSCRAMBLER) : NULL) #define BITSCRAMBLER_LL_INST_LEN_WORDS 9 //length of one instruction in 32-bit words as defined by HW +// LUT index register is 11 bits wide, so the LUT address space is 2048 bytes. +#define BITSCRAMBLER_LL_LUT_MAX_BYTES (1U << 11) +#define BITSCRAMBLER_LL_MAX_INST 8 typedef enum { BITSCRAMBLER_LL_MEM_LP_MODE_DEEP_SLEEP, // memory will enter deep sleep during low power stage, keep memory data