diff --git a/hal/stm32c0.c b/hal/stm32c0.c index 4e703e45..d8605f67 100644 --- a/hal/stm32c0.c +++ b/hal/stm32c0.c @@ -150,7 +150,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { flash_clear_errors(); - if ((len - i > 3) && ((((address + i) & 0x07) == 0) && + if ((len - i >= 8) && ((((address + i) & 0x07) == 0) && ((((uint32_t)data) + i) & 0x07) == 0)) { src = (uint32_t *)data; dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE); diff --git a/hal/stm32g0.c b/hal/stm32g0.c index e55e3594..8becc90f 100644 --- a/hal/stm32g0.c +++ b/hal/stm32g0.c @@ -140,7 +140,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { flash_clear_errors(); - if ((len - i > 3) && ((((address + i) & 0x07) == 0) && + if ((len - i >= 8) && ((((address + i) & 0x07) == 0) && ((((uint32_t)data) + i) & 0x07) == 0)) { src = (uint32_t *)data; dst = (uint32_t *)address; diff --git a/hal/stm32g4.c b/hal/stm32g4.c index 767a4110..08c994b6 100644 --- a/hal/stm32g4.c +++ b/hal/stm32g4.c @@ -54,7 +54,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { flash_clear_errors(); - if ((len - i > 3) && ((((address + i) & 0x07) == 0) && + if ((len - i >= 8) && ((((address + i) & 0x07) == 0) && ((((uint32_t)data) + i) & 0x07) == 0)) { src = (uint32_t *)data; dst = (uint32_t *)address; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 997e02df..1e204bae 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -115,6 +115,7 @@ TESTS+=unit-t10xx-qe-firmware TESTS+=unit-aurix-erased-fill TESTS+=unit-aurix-erased-fill-invert TESTS+=unit-t2080-fman-loader +TESTS+=unit-stm32g4-write TESTS+=unit-stm32l5-write TESTS+=unit-stm32u5-write TESTS+=unit-nvm-cache-scrub @@ -989,6 +990,19 @@ t2080_fman_extract.h: ../../hal/nxp_t2080.c unit-t2080-fman-loader: unit-t2080-fman-loader.c t2080_fman_extract.h gcc -o $@ unit-t2080-fman-loader.c $(CFLAGS) $(LDFLAGS) +# unit-stm32g4-write runs the real hal_flash_write() from hal/stm32g4.c +# (F-11023: the double-word fast path was selected on "len - i > 3" but +# consumed eight bytes, so an aligned 4-7 byte tail over-read the +# caller's buffer and over-programmed flash). Same harness as the +# STM32L5/STM32U5 twins; the g4 helpers are static and un-prefixed. +stm32g4_write_extract.h: ../../hal/stm32g4.c + sed -n '/^static RAMFUNCTION void flash_wait_complete/,/^}/p' $< > $@ + sed -n '/^static void RAMFUNCTION flash_clear_errors/,/^}/p' $< >> $@ + sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< >> $@ + +unit-stm32g4-write: unit-stm32g4-write.c stm32g4_write_extract.h + gcc -o $@ unit-stm32g4-write.c $(CFLAGS) $(LDFLAGS) + # unit-stm32l5-write runs the real hal_flash_write() from hal/stm32l5.c # (an 8-byte program unit was read whole even when len left a # partial unit, over-reading the caller's buffer and writing the excess @@ -1157,7 +1171,8 @@ covclean: # so "clean" removes them and so there is one place that names them. GENERATED_SRC:=aurix_erased_extract.h nvm_cache_scrub_extract.h \ nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \ - sdhci_host.c stm32l5_write_extract.h stm32u5_write_extract.h \ + sdhci_host.c stm32g4_write_extract.h stm32l5_write_extract.h \ + stm32u5_write_extract.h \ t10xx_qe_firmware_extract.h t2080_fman_extract.h \ ti_hercules_write_extract.h versal_ext_write_extract.h versal_host.c \ versal_host.h versal_qspidev_extract.h zynq_erase_extract.h \ diff --git a/tools/unit-tests/unit-stm32g4-write.c b/tools/unit-tests/unit-stm32g4-write.c new file mode 100644 index 00000000..c6eb00ae --- /dev/null +++ b/tools/unit-tests/unit-stm32g4-write.c @@ -0,0 +1,216 @@ +/* unit-stm32g4-write.c + * + * Regression test for F-11023: the double-word fast path of + * hal_flash_write() in hal/stm32g4.c was selected on "len - i > 3" + * but consumed eight bytes, so an aligned 4-7 byte tail read up to + * four bytes past the caller's buffer and programmed them into + * flash. The fix requires at least eight remaining bytes before + * taking the fast path; shorter tails fall to the RMW branch, which + * rewrites the unit with the out-of-range bytes read back from + * flash. + * + * Same harness as the STM32L5/STM32U5 twins: extracted functions, + * registers on a host file, stale destination flash, canary after + * the source. The source buffer is 8-byte aligned so the fast-path + * alignment test on the data pointer can pass. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include + +/* Host stand-in for the ARM build attribute. */ +#define RAMFUNCTION + +/* Host FLASH register file (offsets as in hal/stm32g4.h). */ +static uint32_t g_flash_regs[0x20 / sizeof(uint32_t)]; +#define FLASH_BASE ((uintptr_t)g_flash_regs) +#define FLASH_SR (*(volatile uint32_t *)(FLASH_BASE + 0x10)) +#define FLASH_CR (*(volatile uint32_t *)(FLASH_BASE + 0x14)) +#define FLASH_SR_EOP (1 << 0) +#define FLASH_SR_OPERR (1 << 1) +#define FLASH_SR_PROGERR (1 << 3) +#define FLASH_SR_WRPERR (1 << 4) +#define FLASH_SR_PGAERR (1 << 5) +#define FLASH_SR_SIZERR (1 << 6) +#define FLASH_SR_PGSERR (1 << 7) +#define FLASH_SR_MISERR (1 << 8) +#define FLASH_SR_FASTERR (1 << 9) +#define FLASH_SR_RDERR (1 << 14) +#define FLASH_SR_OPTVERR (1 << 15) +#define FLASH_SR_BSY (1 << 16) +#define FLASH_CR_PG (1 << 0) + +/* Destination flash: pre-filled with stale data (rewrite scenario). + * hal_flash_write() takes the address as uint32_t (32-bit MCU), so on + * the 64-bit host the flash must live at an address that fits in 32 + * bits: map it at a fixed low location. */ +#define FLASH_MEM_SZ 256 +#define FLASH_MEM_ADDR 0x10000000UL +static uint8_t *g_flash_mem; + +/* Source buffer followed by a canary: a pre-fix short write reads the + * canary and lands it in the destination flash. */ +#define DATA_SZ 64 +#define CANARY_SZ 32 +static uint8_t g_data[DATA_SZ + CANARY_SZ] __attribute__((aligned(8))); +#define g_canary (g_data + DATA_SZ) + +/* The real functions from hal/stm32g4.c (extracted by the Makefile). */ +#include "stm32g4_write_extract.h" + +static void setup(void) +{ + int i; + + memset(g_flash_regs, 0, sizeof(g_flash_regs)); + for (i = 0; i < FLASH_MEM_SZ; i++) + g_flash_mem[i] = 0x12; /* stale */ + for (i = 0; i < DATA_SZ; i++) + g_data[i] = (uint8_t)(0x30 + i); + /* 0x70..0x8F: distinct from the data bytes (0x30..0x6F), the stale + * flash fill (0x12) and the erased-value padding (0xFF), so a + * canary hit means source bytes past len were really read. */ + for (i = 0; i < CANARY_SZ; i++) + g_canary[i] = (uint8_t)(0x70 + i); +} + +static void teardown(void) +{ +} + +static int canary_in_flash(void) +{ + int i; + + for (i = 0; i < CANARY_SZ; i++) + if (memchr(g_flash_mem, g_canary[i], FLASH_MEM_SZ) != NULL) + return 1; + return 0; +} + +/* A write of 60 bytes: seven full double words, then a 4-byte tail. + * Pre-fix the tail took the fast path and programmed bytes 60..63 + * from source bytes past len. Post-fix the tail is RMW'd and nothing + * past len is read or written. */ +START_TEST(test_write_60_no_overread) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 60), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 60), 0); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 58 bytes: the final unit is partial (bytes 58,59 are + * outside the request); they are read back from flash and rewritten + * unchanged, and nothing past len is read. */ +START_TEST(test_write_58_partial_word_padded) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 58), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 58), 0); + /* word 14 (bytes 56..59): 58,59 keep their flash content */ + ck_assert_uint_eq(g_flash_mem[58], 0x12); + ck_assert_uint_eq(g_flash_mem[59], 0x12); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 3 bytes: the whole 8-byte unit is programmed, but only + * bytes 0..2 take the requested value; the rest is rewritten with + * what flash already held. */ +START_TEST(test_write_3_single_word_padded) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 3), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 3), 0); + /* byte 3 and the whole second word are rewritten unchanged */ + ck_assert_uint_eq(g_flash_mem[3], 0x12); + for (i = 4; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 64 bytes, a multiple of 8: the fast path is taken for + * every unit and behaves exactly as before the fix. */ +START_TEST(test_write_64_full_units) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 64), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 64), 0); + for (i = 64; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); +} +END_TEST + +Suite *stm32g4_write_suite(void) +{ + Suite *s = suite_create("stm32g4-write"); + TCase *tc = tcase_create("stm32g4-write"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_write_60_no_overread); + tcase_add_test(tc, test_write_58_partial_word_padded); + tcase_add_test(tc, test_write_3_single_word_padded); + tcase_add_test(tc, test_write_64_full_units); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = stm32g4_write_suite(); + SRunner *sr = srunner_create(s); + + g_flash_mem = mmap((void *)FLASH_MEM_ADDR, FLASH_MEM_SZ, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, + -1, 0); + if (g_flash_mem == MAP_FAILED) + return 99; + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + munmap(g_flash_mem, FLASH_MEM_SZ); + + return fails; +}