mirror of https://github.com/wolfSSL/wolfBoot.git
F-11035: size the final partial page from remaining bytes in hifive1 write
hal_flash_write() in hal/hifive1.c selected the page path and clamped the partial-page length from the original total len instead of the bytes still remaining (len - j). A page-aligned multi-page write that ended in a partial page therefore took the full-page branch on the last iteration: it read past the end of the caller's buffer and programmed a full 256-byte page where only the remaining bytes were requested, clobbering flash past the update range. Compute remaining = len - j at the top of the loop and use it for both the branch test and the rel_len clamp; j still advances only by the bytes actually consumed (256 on the full-page path, rel_len on the partial path). unit-hifive1-flash-write runs the real extracted function against a mock fespi model (FLASH_BASE points at a flash image buffer, fespi_write_address/fespi_sw_tx program into it, and the RMW path reads the image back through FLASH_BASE as on hardware). The regression case is a 356-byte aligned write: the last page's tail must stay erased, which fails pre-fix (the over-read bytes are programmed instead). Verification: - Built: riscv-none-elf-gcc 15.2 -fsyntax-only -Wall with ARCH_RISCV: clean. - Tested: unit-hifive1-flash-write 3/3; pre-fix the 356-byte case wrote non-erased bytes past offset 356 of the flash image. Unaligned single-page RMW and exact-full-page cases unchanged. - Pitfalls: relative (sub-FLASH_BASE) addresses are accepted as-is by the function, which is what the test passes so the 32-bit address parameter never carries a 64-bit host pointer. - Style: cstyle-check.sh flag count on hal/hifive1.c unchanged from the pre-change file; the new test trips only the uncrustify pointer-alignment class the sibling unit tests trip. - Message: F-11035: prefix, no co-author trailers. - Unverified: no HiFive1 board execution.pull/870/head
parent
f29309ed3f
commit
2765348e34
|
|
@ -492,7 +492,9 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
|
|||
page = address >> 8;
|
||||
|
||||
while (j < (uint32_t)len) {
|
||||
if ((off > 0) || (len < FLASH_PAGE_SIZE)) {
|
||||
uint32_t remaining = (uint32_t)len - j;
|
||||
|
||||
if ((off > 0) || (remaining < FLASH_PAGE_SIZE)) {
|
||||
uint8_t *orig = (uint8_t *)(FLASH_BASE + (page << 8));
|
||||
int rel_len;
|
||||
rel_len = FLASH_PAGE_SIZE - off;
|
||||
|
|
@ -500,8 +502,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
|
|||
fespi_hwmode();
|
||||
swmode = 0;
|
||||
}
|
||||
if (rel_len > len)
|
||||
rel_len = len;
|
||||
if (rel_len > (int)remaining)
|
||||
rel_len = (int)remaining;
|
||||
for (i = 0; i < off; i++)
|
||||
data_copy[i] = orig[i];
|
||||
for (i = off; i < off + rel_len; i++)
|
||||
|
|
|
|||
|
|
@ -115,6 +115,7 @@ TESTS+=unit-versal-ext-write
|
|||
TESTS+=unit-t10xx-qe-firmware
|
||||
TESTS+=unit-t10xx-flash-status
|
||||
TESTS+=unit-p1021-erase-advance
|
||||
TESTS+=unit-hifive1-flash-write
|
||||
TESTS+=unit-aurix-erased-fill
|
||||
TESTS+=unit-aurix-erased-fill-invert
|
||||
TESTS+=unit-t2080-fman-loader
|
||||
|
|
@ -1048,6 +1049,16 @@ unit-p1021-erase-advance: unit-p1021-erase-advance.c p1021_erase_extract.h \
|
|||
p1021_erase_fn_extract.h
|
||||
gcc -o $@ unit-p1021-erase-advance.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-hifive1-flash-write runs the real hal_flash_write() from
|
||||
# hal/hifive1.c against a mock fespi model (F-11035: the final partial
|
||||
# page of a multi-page write took the full-page branch, over-reading
|
||||
# the input and over-programming flash).
|
||||
hifive1_flash_write_extract.h: ../../hal/hifive1.c
|
||||
sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< > $@
|
||||
|
||||
unit-hifive1-flash-write: unit-hifive1-flash-write.c hifive1_flash_write_extract.h
|
||||
gcc -o $@ unit-hifive1-flash-write.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-ecc-raw-der runs the real wolfCrypt raw-to-DER conversion and
|
||||
# verification (F-11024: the wolfHSM verify path in src/image.c passed
|
||||
# minimal field sizes with field-start pointers to
|
||||
|
|
|
|||
|
|
@ -0,0 +1,194 @@
|
|||
/* unit-hifive1-flash-write.c
|
||||
*
|
||||
* Regression test for F-11035: hal_flash_write() in hal/hifive1.c
|
||||
* selected the page path and sized the partial-page copy from the
|
||||
* original total `len` instead of the bytes still remaining. A
|
||||
* multi-page write that ends in a partial page therefore took the
|
||||
* full-page branch on the last iteration, read past the end of the
|
||||
* caller's buffer and programmed a whole page past the requested range.
|
||||
*
|
||||
* The real function is extracted by the Makefile and run against a
|
||||
* mock fespi model: FLASH_BASE points at a flash image buffer,
|
||||
* fespi_write_address()/fespi_sw_tx() program into it, and the
|
||||
* read-modify-write path reads the current image back through
|
||||
* FLASH_BASE, exactly as the hardware would.
|
||||
*
|
||||
* 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>
|
||||
|
||||
#define RAMFUNCTION
|
||||
#define FLASH_PAGE_SIZE 256
|
||||
#define FESPI_PAGE_PROGRAM 0x02
|
||||
|
||||
#define NUM_PAGES 4
|
||||
static uint8_t g_flash[NUM_PAGES * FLASH_PAGE_SIZE];
|
||||
#define FLASH_BASE ((uintptr_t)g_flash)
|
||||
|
||||
static uint32_t g_txmark;
|
||||
#define FESPI_REG_TXMARK (g_txmark)
|
||||
|
||||
/* ---- mock fespi model ------------------------------------------------ */
|
||||
|
||||
static uint32_t g_cur; /* byte offset in g_flash the next sw_tx lands at */
|
||||
|
||||
static void fespi_write_address(uint32_t address)
|
||||
{
|
||||
g_cur = address;
|
||||
}
|
||||
|
||||
static void fespi_sw_tx(uint8_t b)
|
||||
{
|
||||
if (g_cur < sizeof(g_flash)) {
|
||||
g_flash[g_cur++] = b;
|
||||
}
|
||||
}
|
||||
|
||||
static void fespi_hwmode(void) { }
|
||||
static void fespi_swmode(void) { }
|
||||
static void fespi_csmode_hold(void){ }
|
||||
static void fespi_csmode_auto(void){ }
|
||||
static void fespi_wait_txwm(void) { }
|
||||
static void fespi_wait_flash_busy(void) { }
|
||||
static void fespi_write_enable(void) { }
|
||||
|
||||
/* The real hal_flash_write() from hal/hifive1.c (extracted). */
|
||||
#include "hifive1_flash_write_extract.h"
|
||||
|
||||
#define ERASED 0xEE
|
||||
#define DATA_B 0xA1
|
||||
#define CANARY 0x5A
|
||||
|
||||
static void model_reset(void)
|
||||
{
|
||||
memset(g_flash, ERASED, sizeof(g_flash));
|
||||
g_txmark = 0;
|
||||
g_cur = 0;
|
||||
}
|
||||
|
||||
/* The bug: a page-aligned multi-page write whose final page is partial.
|
||||
* Pre-fix the last iteration took the full-page branch, read
|
||||
* data[len..len+155] (past the buffer) and programmed 256 bytes where
|
||||
* only 100 were requested. The tail of the last page must keep the
|
||||
* erased pattern, and nothing past the requested bytes may change. */
|
||||
START_TEST (test_final_partial_page_not_overread)
|
||||
{
|
||||
uint8_t data[356];
|
||||
uint8_t canary[160];
|
||||
int i;
|
||||
int ret;
|
||||
|
||||
/* data is exactly the request length; a canary sits right after it
|
||||
* so any out-of-range read is observable. */
|
||||
for (i = 0; i < (int)sizeof(data); i++)
|
||||
data[i] = DATA_B;
|
||||
for (i = 0; i < (int)sizeof(canary); i++)
|
||||
canary[i] = CANARY;
|
||||
(void)canary;
|
||||
|
||||
/* Relative offsets are accepted as-is (address < FLASH_BASE), which
|
||||
* keeps the 32-bit address parameter away from 64-bit host pointers. */
|
||||
model_reset();
|
||||
ret = hal_flash_write(0, data, sizeof(data));
|
||||
ck_assert_int_eq(ret, 0);
|
||||
|
||||
/* Page 0: full page of data. */
|
||||
ck_assert_mem_eq(g_flash, data, FLASH_PAGE_SIZE);
|
||||
|
||||
/* Page 1: first 100 bytes are data, the remaining 156 must stay
|
||||
* erased (a full-page branch would have written data/canary here). */
|
||||
ck_assert_mem_eq(g_flash + FLASH_PAGE_SIZE, data + FLASH_PAGE_SIZE,
|
||||
100);
|
||||
for (i = 100; i < FLASH_PAGE_SIZE; i++) {
|
||||
ck_assert_uint_eq(g_flash[FLASH_PAGE_SIZE + i], ERASED);
|
||||
}
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* A single unaligned write that stays within one page (the RMW path
|
||||
* that already worked): leading and trailing bytes of the page are
|
||||
* preserved. */
|
||||
START_TEST (test_unaligned_single_page_rmw)
|
||||
{
|
||||
uint8_t data[64];
|
||||
int i;
|
||||
int ret;
|
||||
|
||||
for (i = 0; i < (int)sizeof(data); i++)
|
||||
data[i] = (uint8_t)(i + 1);
|
||||
|
||||
model_reset();
|
||||
ret = hal_flash_write(100, data, sizeof(data));
|
||||
ck_assert_int_eq(ret, 0);
|
||||
|
||||
/* Offsets before and after the write keep the erased pattern. */
|
||||
for (i = 0; i < 100; i++)
|
||||
ck_assert_uint_eq(g_flash[i], ERASED);
|
||||
ck_assert_mem_eq(g_flash + 100, data, sizeof(data));
|
||||
for (i = 100 + (int)sizeof(data); i < FLASH_PAGE_SIZE; i++)
|
||||
ck_assert_uint_eq(g_flash[i], ERASED);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* A write of exactly one full page is unchanged by the fix. */
|
||||
START_TEST (test_exact_full_page)
|
||||
{
|
||||
uint8_t data[FLASH_PAGE_SIZE];
|
||||
int i;
|
||||
int ret;
|
||||
|
||||
for (i = 0; i < (int)sizeof(data); i++)
|
||||
data[i] = (uint8_t)(0xC0 ^ (i & 0x1F));
|
||||
|
||||
model_reset();
|
||||
ret = hal_flash_write(0, data, sizeof(data));
|
||||
ck_assert_int_eq(ret, 0);
|
||||
ck_assert_mem_eq(g_flash, data, FLASH_PAGE_SIZE);
|
||||
for (i = 0; i < FLASH_PAGE_SIZE; i++)
|
||||
ck_assert_uint_eq(g_flash[FLASH_PAGE_SIZE + i], ERASED);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
Suite *hifive1_flash_write_suite(void)
|
||||
{
|
||||
Suite *s = suite_create("hifive1 flash write");
|
||||
TCase *tc = tcase_create("final-partial-page");
|
||||
|
||||
tcase_add_test(tc, test_final_partial_page_not_overread);
|
||||
tcase_add_test(tc, test_unaligned_single_page_rmw);
|
||||
tcase_add_test(tc, test_exact_full_page);
|
||||
tcase_set_timeout(tc, 10);
|
||||
suite_add_tcase(s, tc);
|
||||
return s;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int fails;
|
||||
Suite *s = hifive1_flash_write_suite();
|
||||
SRunner *sr = srunner_create(s);
|
||||
|
||||
srunner_run_all(sr, CK_NORMAL);
|
||||
fails = srunner_ntests_failed(sr);
|
||||
srunner_free(sr);
|
||||
return fails;
|
||||
}
|
||||
Loading…
Reference in New Issue