From 7461a9f900025c728702948c6b2811e36e814b7d Mon Sep 17 00:00:00 2001 From: yi chen <94xhn1@gmail.com> Date: Sun, 12 Jul 2026 01:02:21 +0800 Subject: [PATCH 1/2] fix(wear_levelling): guard WL_Flash::write()/read() against size==0 underflow WL_Flash::write() and WL_Flash::read() computed: uint32_t count = (size - 1) / this->cfg.wl_page_size; `size` is `size_t` (unsigned). Neither the public wl_write()/wl_read() API (wear_levelling.cpp), nor the newer wl_bdl_write()/wl_bdl_read() block-device path (wl_blockdev.cpp), reject size == 0 before calling into WL_Flash, and wear_levelling.h does not document size == 0 as invalid (a 0-byte write/read is a reasonable no-op, mirroring POSIX write()/read() with count == 0). When size == 0, `size - 1` wraps around to SIZE_MAX, so `count` becomes an enormous page count instead of 0. The functions then loop that many times, reading (write()) or writing (read()) `wl_page_size` bytes per iteration through the flash partition, immediately walking past the caller-supplied buffer on the very first iteration: - write(): out-of-bounds *read* from the caller's `src` buffer. - read(): out-of-bounds *write* into the caller's `dest` buffer -- the more severe case, since it corrupts caller memory with flash content instead of merely over-reading. Verified with a standalone reproduction that compiles the unmodified WL_Flash.cpp against a mock Flash_Access partition: calling `wl.write(0, an_8_byte_buffer, 0)` with no other change immediately segfaults (confirmed count == 0xFFFFFFFF for wl_page_size == 4096); with this fix applied the same call returns ESP_OK without touching memory outside the buffer, and normal non-zero-size read/write is unaffected. Add an early `size == 0` return (mirroring the existing `!initialized` guard) to both functions, and a host_test regression case exercising wl_write()/wl_read() with size == 0 through the public API. Disclosure: this fix was prepared with AI assistance (Claude) and reviewed by me before submission. Signed-off-by: yi chen <94xhn1@gmail.com> --- components/wear_levelling/WL_Flash.cpp | 12 +++++++ .../wear_levelling/host_test/main/test_wl.cpp | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/components/wear_levelling/WL_Flash.cpp b/components/wear_levelling/WL_Flash.cpp index c64af17c9f3..af255271942 100644 --- a/components/wear_levelling/WL_Flash.cpp +++ b/components/wear_levelling/WL_Flash.cpp @@ -575,6 +575,13 @@ esp_err_t WL_Flash::write(size_t dest_addr, const void *src, size_t size) if (!this->initialized) { return ESP_ERR_INVALID_STATE; } + if (size == 0) { + // Nothing to do. Guard this explicitly: size is unsigned, so + // `size - 1` below would otherwise wrap around to SIZE_MAX and turn + // "count" into a huge page count, walking far past the caller's + // buffer (see components/wear_levelling/host_test). + return ESP_OK; + } ESP_LOGD(TAG, "%s - dest_addr= 0x%08" PRIx32 ", size= 0x%08" PRIx32 , __func__, (uint32_t) dest_addr, (uint32_t) size); uint32_t count = (size - 1) / this->cfg.wl_page_size; for (size_t i = 0; i < count; i++) { @@ -594,6 +601,11 @@ esp_err_t WL_Flash::read(size_t src_addr, void *dest, size_t size) if (!this->initialized) { return ESP_ERR_INVALID_STATE; } + if (size == 0) { + // See the matching guard in WL_Flash::write() above: size==0 must + // not be allowed to reach the `size - 1` computation below. + return ESP_OK; + } ESP_LOGD(TAG, "%s - src_addr= 0x%08" PRIx32 ", size= 0x%08" PRIx32 , __func__, (uint32_t) src_addr, (uint32_t) size); uint32_t count = (size - 1) / this->cfg.wl_page_size; for (size_t i = 0; i < count; i++) { diff --git a/components/wear_levelling/host_test/main/test_wl.cpp b/components/wear_levelling/host_test/main/test_wl.cpp index 69ac2634f30..ce9819a8743 100644 --- a/components/wear_levelling/host_test/main/test_wl.cpp +++ b/components/wear_levelling/host_test/main/test_wl.cpp @@ -99,6 +99,39 @@ TEST_CASE("write and read back data", "[wear_levelling]") free(read); } +TEST_CASE("write and read with zero size are safe no-ops", "[wear_levelling]") +{ + esp_err_t result; + wl_handle_t wl_handle; + + const esp_partition_t *partition = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_ANY, "storage"); + + // Mount wear-levelled partition + result = wl_mount(partition, &wl_handle); + REQUIRE(result == ESP_OK); + + // wl_write()/wl_read() do not document size==0 as invalid (analogous to + // POSIX write()/read() with count==0), so it must not be treated as an + // out-of-bounds request. Previously, WL_Flash::write()/read() computed + // `(size - 1) / wl_page_size` without checking for size==0 first; since + // size is unsigned, size==0 wrapped this to a huge page count and walked + // far past the caller-provided buffer. + uint8_t dummy = 0xAA; + result = wl_write(wl_handle, 0, &dummy, 0); + REQUIRE(result == ESP_OK); + + uint8_t read_back = 0x55; + result = wl_read(wl_handle, 0, &read_back, 0); + REQUIRE(result == ESP_OK); + + // Untouched by a genuine zero-length read. + REQUIRE(read_back == 0x55); + + // Unmount + result = wl_unmount(wl_handle); + REQUIRE(result == ESP_OK); +} + TEST_CASE("power down test", "[wear_levelling]") { esp_err_t result; From 2039a586fd556786d58db01a4fa40bd5f975435a Mon Sep 17 00:00:00 2001 From: "sonika.rathi" Date: Thu, 16 Jul 2026 09:29:59 +0200 Subject: [PATCH 2/2] fix(wear_levelling): shorten zero-size guard comments --- components/wear_levelling/WL_Flash.cpp | 8 ++------ components/wear_levelling/host_test/main/test_wl.cpp | 7 +------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/components/wear_levelling/WL_Flash.cpp b/components/wear_levelling/WL_Flash.cpp index af255271942..9ad1da2ce5f 100644 --- a/components/wear_levelling/WL_Flash.cpp +++ b/components/wear_levelling/WL_Flash.cpp @@ -576,10 +576,7 @@ esp_err_t WL_Flash::write(size_t dest_addr, const void *src, size_t size) return ESP_ERR_INVALID_STATE; } if (size == 0) { - // Nothing to do. Guard this explicitly: size is unsigned, so - // `size - 1` below would otherwise wrap around to SIZE_MAX and turn - // "count" into a huge page count, walking far past the caller's - // buffer (see components/wear_levelling/host_test). + // size==0: (size-1) unsigned underflow would OOB the caller buffer. return ESP_OK; } ESP_LOGD(TAG, "%s - dest_addr= 0x%08" PRIx32 ", size= 0x%08" PRIx32 , __func__, (uint32_t) dest_addr, (uint32_t) size); @@ -602,8 +599,7 @@ esp_err_t WL_Flash::read(size_t src_addr, void *dest, size_t size) return ESP_ERR_INVALID_STATE; } if (size == 0) { - // See the matching guard in WL_Flash::write() above: size==0 must - // not be allowed to reach the `size - 1` computation below. + // Same size==0 guard as write(); avoid (size-1) underflow below. return ESP_OK; } ESP_LOGD(TAG, "%s - src_addr= 0x%08" PRIx32 ", size= 0x%08" PRIx32 , __func__, (uint32_t) src_addr, (uint32_t) size); diff --git a/components/wear_levelling/host_test/main/test_wl.cpp b/components/wear_levelling/host_test/main/test_wl.cpp index ce9819a8743..012945df5ab 100644 --- a/components/wear_levelling/host_test/main/test_wl.cpp +++ b/components/wear_levelling/host_test/main/test_wl.cpp @@ -110,12 +110,7 @@ TEST_CASE("write and read with zero size are safe no-ops", "[wear_levelling]") result = wl_mount(partition, &wl_handle); REQUIRE(result == ESP_OK); - // wl_write()/wl_read() do not document size==0 as invalid (analogous to - // POSIX write()/read() with count==0), so it must not be treated as an - // out-of-bounds request. Previously, WL_Flash::write()/read() computed - // `(size - 1) / wl_page_size` without checking for size==0 first; since - // size is unsigned, size==0 wrapped this to a huge page count and walked - // far past the caller-provided buffer. + // Zero-length wl_write/read must be no-ops; size==0 used to underflow (size-1) and OOB the buffer. uint8_t dummy = 0xAA; result = wl_write(wl_handle, 0, &dummy, 0); REQUIRE(result == ESP_OK);