From 97f9a36b2d7df1853dd7cc884f31f8786410cf11 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 13 Jul 2026 15:20:57 +0900 Subject: [PATCH 1/3] fix(esp_timer): add target test for long dump lines --- .../esp_timer/test_apps/main/test_esp_timer.c | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/components/esp_timer/test_apps/main/test_esp_timer.c b/components/esp_timer/test_apps/main/test_esp_timer.c index fce31680b9e..6b670aeaf81 100644 --- a/components/esp_timer/test_apps/main/test_esp_timer.c +++ b/components/esp_timer/test_apps/main/test_esp_timer.c @@ -506,6 +506,7 @@ TEST_CASE("Can dump esp_timer stats", "[esp_timer]") overflow the internal string buffer if the length calculation is not correct. */ +#if CONFIG_IDF_TARGET_LINUX const int NUM_TIMERS = 200; esp_timer_handle_t timers[NUM_TIMERS]; @@ -525,6 +526,57 @@ TEST_CASE("Can dump esp_timer stats", "[esp_timer]") for (int i = 0; i < NUM_TIMERS; ++i) { TEST_ESP_OK(esp_timer_delete(timers[i])); } +#else + enum { + INACTIVE_TIMER_COUNT = 64, + NUM_TIMERS = 256, + }; + const uint64_t long_period = (1ULL << 56) - 1; + const uint64_t long_alarm = (uint64_t)esp_timer_get_time() + 1000000000000ULL; + /* Keep large test buffers off the Unity task stack on smaller targets. */ + static esp_timer_handle_t inactive_timers[INACTIVE_TIMER_COUNT]; + static char inactive_names[INACTIVE_TIMER_COUNT][30]; + static esp_timer_handle_t timers[NUM_TIMERS]; + + for (size_t i = 0; i < INACTIVE_TIMER_COUNT; ++i) { + snprintf(inactive_names[i], sizeof(inactive_names[i]), "test_timer_number_%zu", i); + const esp_timer_create_args_t timer_args = { + .callback = &empty_cb, + .arg = NULL, + .name = inactive_names[i], + }; + TEST_ESP_OK(esp_timer_create(&timer_args, &inactive_timers[i])); + } + + for (size_t i = 0; i < NUM_TIMERS; ++i) { + const esp_timer_create_args_t timer_args = { + .callback = &empty_cb, + .name = "dump_long_line_timer", + }; + TEST_ESP_OK(esp_timer_create(&timer_args, &timers[i])); + TEST_ESP_OK(esp_timer_start_periodic_at(timers[i], long_period, long_alarm + i)); + } + + static char dump_buf[4096]; + memset(dump_buf, 0, sizeof(dump_buf)); + FILE* stream = fmemopen(dump_buf, sizeof(dump_buf) - 1, "w+"); + TEST_ASSERT_NOT_NULL(stream); + TEST_ESP_OK(esp_timer_dump(stream)); + fflush(stream); + fclose(stream); + TEST_ASSERT_NOT_NULL(strstr(dump_buf, "Timer stats:")); + TEST_ASSERT_NOT_NULL(strstr(dump_buf, "72057594037927935")); + TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true)); + + for (size_t i = 0; i < NUM_TIMERS; ++i) { + TEST_ESP_OK(esp_timer_stop(timers[i])); + TEST_ESP_OK(esp_timer_delete(timers[i])); + } + + for (size_t i = 0; i < INACTIVE_TIMER_COUNT; ++i) { + TEST_ESP_OK(esp_timer_delete(inactive_timers[i])); + } +#endif } typedef struct { From 21c4f307de66a80d96bfce29a60bcf4b1c863de3 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 13 Jul 2026 15:45:00 +0900 Subject: [PATCH 2/3] fix(esp_timer): avoid dump buffer overflow print_timer_info advanced the dump cursor by snprintf's return value. When a timer line was truncated, snprintf returned the full would-be length, which could move the cursor past the heap buffer and wrap the remaining size before the next write. Add a bounded append helper that clamps truncation to the end of the buffer while preserving the NUL terminator. --- components/esp_timer/src/esp_timer.c | 46 ++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/components/esp_timer/src/esp_timer.c b/components/esp_timer/src/esp_timer.c index cf8d8abd83c..9d0c2db5f2c 100644 --- a/components/esp_timer/src/esp_timer.c +++ b/components/esp_timer/src/esp_timer.c @@ -5,6 +5,7 @@ */ #include +#include #include #include "soc/soc.h" #include "esp_types.h" @@ -695,28 +696,54 @@ esp_err_t esp_timer_deinit(void) return ESP_OK; } +static void append_to_buffer(char** dst, size_t* dst_size, const char* format, ...) +{ + /* Defensive: avoid underflow in the truncation path below if no space remains. */ + if (*dst_size == 0) { + return; + } + + va_list args; + va_start(args, format); + int cb = vsnprintf(*dst, *dst_size, format, args); + va_end(args); + + if (cb < 0) { + return; + } + + /* On truncation, snprintf returns the full would-be length. Keep the + * cursor inside the buffer and preserve the terminating NUL byte. + */ + if ((size_t) cb >= *dst_size) { + *dst += *dst_size - 1; + *dst_size = 1; + return; + } + + *dst += cb; + *dst_size -= cb; +} + static void print_timer_info(esp_timer_handle_t t, char** dst, size_t* dst_size) { #if WITH_PROFILING - size_t cb; // name is optional, might be missed. if (t->name) { - cb = snprintf(*dst, *dst_size, "%-20.20s ", t->name); + append_to_buffer(dst, dst_size, "%-20.20s ", t->name); } else { - cb = snprintf(*dst, *dst_size, "timer@%-10p ", t); + append_to_buffer(dst, dst_size, "timer@%-10p ", t); } - cb += snprintf(*dst + cb, *dst_size - cb, "%-10lld %-12lld %-12d %-12d %-12d %-12lld\n", - (uint64_t)t->period, t->alarm, t->times_armed, - t->times_triggered, t->times_skipped, t->total_callback_run_time); + append_to_buffer(dst, dst_size, "%-10lld %-12lld %-12d %-12d %-12d %-12lld\n", + (uint64_t)t->period, t->alarm, t->times_armed, + t->times_triggered, t->times_skipped, t->total_callback_run_time); /* keep this in sync with the format string, used in esp_timer_dump */ #define TIMER_INFO_LINE_LEN 103 #else - size_t cb = snprintf(*dst, *dst_size, "timer@%-14p %-10lld %-12lld\n", t, (uint64_t)t->period, t->alarm); + append_to_buffer(dst, dst_size, "timer@%-14p %-10lld %-12lld\n", t, (uint64_t)t->period, t->alarm); #define TIMER_INFO_LINE_LEN 47 #endif - *dst += cb; - *dst_size -= cb; } esp_err_t esp_timer_dump(FILE* stream) @@ -750,6 +777,7 @@ esp_err_t esp_timer_dump(FILE* stream) * slightly more and the output will be truncated if that is not enough. */ size_t buf_size = TIMER_INFO_LINE_LEN * (timer_count + 3); + /* buf_size is the snprintf size, including NUL; keep one extra byte as slack. */ char* print_buf = calloc(1, buf_size + 1); if (print_buf == NULL) { return ESP_ERR_NO_MEM; From abc386e15428024341fbf58d27e6c0f33311fd02 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 13 Jul 2026 15:52:13 +0900 Subject: [PATCH 3/3] fix(esp_timer): enable dump overflow test on linux --- .../esp_timer/test_apps/main/CMakeLists.txt | 9 +++++++ .../esp_timer/test_apps/main/test_esp_timer.c | 25 +++---------------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/components/esp_timer/test_apps/main/CMakeLists.txt b/components/esp_timer/test_apps/main/CMakeLists.txt index 53c4d8eed69..2b3721d0163 100644 --- a/components/esp_timer/test_apps/main/CMakeLists.txt +++ b/components/esp_timer/test_apps/main/CMakeLists.txt @@ -23,3 +23,12 @@ idf_component_register(SRCS ${srcs} PRIV_INCLUDE_DIRS "../../private_include" "../include" PRIV_REQUIRES cmock test_utils esp_timer spi_flash esp_psram esp_driver_gpio esp_pm WHOLE_ARCHIVE) + +if(CONFIG_IDF_TARGET_LINUX AND NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin") + # Linux host heap integrity checks are stubs, so use ASan to catch dump overflows. + set(asan_options -fsanitize=address -fno-omit-frame-pointer) + idf_component_get_property(esp_timer_lib esp_timer COMPONENT_LIB) + target_compile_options(${esp_timer_lib} PRIVATE ${asan_options}) + target_compile_options(${COMPONENT_LIB} PRIVATE ${asan_options}) + idf_build_set_property(LINK_OPTIONS -fsanitize=address APPEND) +endif() diff --git a/components/esp_timer/test_apps/main/test_esp_timer.c b/components/esp_timer/test_apps/main/test_esp_timer.c index 6b670aeaf81..993273c9a06 100644 --- a/components/esp_timer/test_apps/main/test_esp_timer.c +++ b/components/esp_timer/test_apps/main/test_esp_timer.c @@ -6,6 +6,7 @@ #include "sdkconfig.h" #include #include +#include #include #include #include @@ -506,27 +507,6 @@ TEST_CASE("Can dump esp_timer stats", "[esp_timer]") overflow the internal string buffer if the length calculation is not correct. */ -#if CONFIG_IDF_TARGET_LINUX - const int NUM_TIMERS = 200; - esp_timer_handle_t timers[NUM_TIMERS]; - - for (int i = 0; i < NUM_TIMERS; ++i) { - char name[30]; - snprintf(name, sizeof(name), "test_timer_number_%d", i); - esp_timer_create_args_t timer_args = { - .callback = &empty_cb, - .arg = NULL, - .name = name - }; - TEST_ESP_OK(esp_timer_create(&timer_args, &timers[i])); - } - - esp_timer_dump(stdout); - - for (int i = 0; i < NUM_TIMERS; ++i) { - TEST_ESP_OK(esp_timer_delete(timers[i])); - } -#else enum { INACTIVE_TIMER_COUNT = 64, NUM_TIMERS = 256, @@ -566,7 +546,9 @@ TEST_CASE("Can dump esp_timer stats", "[esp_timer]") fclose(stream); TEST_ASSERT_NOT_NULL(strstr(dump_buf, "Timer stats:")); TEST_ASSERT_NOT_NULL(strstr(dump_buf, "72057594037927935")); +#if !CONFIG_IDF_TARGET_LINUX TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true)); +#endif for (size_t i = 0; i < NUM_TIMERS; ++i) { TEST_ESP_OK(esp_timer_stop(timers[i])); @@ -576,7 +558,6 @@ TEST_CASE("Can dump esp_timer stats", "[esp_timer]") for (size_t i = 0; i < INACTIVE_TIMER_COUNT; ++i) { TEST_ESP_OK(esp_timer_delete(inactive_timers[i])); } -#endif } typedef struct {