mirror of https://github.com/wolfSSL/wolfBoot.git
F-11023: require a full double word in the STM32 fast write paths
The double-word fast path in hal_flash_write() on STM32G4, STM32C0 and STM32G0 was selected on 'len - i > 3' but programs an 8-byte unit, so an aligned 4-7 byte tail read up to 4 bytes past the caller's buffer and programmed those bytes into flash. Require len - i >= 8 before taking the fast path. Shorter tails fall through to the existing RMW branch, which rewrites the unit with the out-of-range bytes read back from flash, so nothing past len is read or programmed. Add unit-stm32g4-write (same harness as the STM32L5/STM32U5 twins), which fails on the 60-byte tail before the fix.pull/870/head
Daniele Lacamera
parent
527c63007e
commit
639866a50e
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 \
|
||||
|
|
|
|||
|
|
@ -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 <check.h>
|
||||
#include <stdint.h>
|
||||
#include <string.h>
|
||||
#include <sys/mman.h>
|
||||
|
||||
/* 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;
|
||||
}
|
||||
Loading…
Reference in New Issue