mirror of https://github.com/wolfSSL/wolfBoot.git
F-12061: RP2350: RMW partial pages in hal_flash_write
flash_range_program() requires a page-aligned address and a page-multiple length (pico-sdk ROM, invalid_params_if on both). The partition-state path without NVM_FLASH_WRITEONCE issues 1-byte (trailer) and 4-byte (magic) writes that violated the contract on every state transition. Keep the direct-program fast path for page-aligned page-multiple writes; otherwise read the page back from XIP, merge the write, and program the full page. The AND program keeps the trailer flag accumulation intact. Add unit-rp2350-flash-write: runs the extracted hal_flash_write against a mock flash_range_program() that enforces the ROM contract (4/6 checks fail pre-fix, 6/6 pass post-fix).pull/880/head
Daniele Lacamera
parent
95227f823c
commit
5c22c271e8
59
hal/rp2350.c
59
hal/rp2350.c
|
|
@ -225,20 +225,57 @@ void hal_prepare_boot(void)
|
|||
int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
|
||||
{
|
||||
uint8_t cache[WOLFBOOT_SECTOR_SIZE];
|
||||
uint32_t flash_addr = address - XIP_BASE;
|
||||
uint32_t written = 0;
|
||||
uint32_t sz;
|
||||
if (((uintptr_t)data & 0x20000000UL) == 0) {
|
||||
/* Not in RAM: copy to cache before writing */
|
||||
while (written < len) {
|
||||
sz = WOLFBOOT_SECTOR_SIZE;
|
||||
if (sz > (len - written))
|
||||
sz = len - written;
|
||||
memcpy(cache, data + written, sz);
|
||||
flash_range_program(address - XIP_BASE + written, cache, sz);
|
||||
written += sz;
|
||||
uint32_t addr;
|
||||
uint32_t page_off;
|
||||
uint32_t page_addr;
|
||||
uint32_t remaining;
|
||||
|
||||
if (len > 0) {
|
||||
if ((flash_addr & (FLASH_PAGE_SIZE - 1)) == 0 &&
|
||||
((uint32_t)len & (FLASH_PAGE_SIZE - 1)) == 0) {
|
||||
/* Page aligned start, page multiple length: program
|
||||
* directly. */
|
||||
if (((uintptr_t)data & 0x20000000UL) == 0) {
|
||||
/* Not in RAM: copy to cache before writing, XIP is
|
||||
* disabled while the flash is programmed. */
|
||||
while (written < (uint32_t)len) {
|
||||
sz = WOLFBOOT_SECTOR_SIZE;
|
||||
if (sz > (uint32_t)len - written)
|
||||
sz = (uint32_t)len - written;
|
||||
memcpy(cache, data + written, sz);
|
||||
flash_range_program(flash_addr + written, cache, sz);
|
||||
written += sz;
|
||||
}
|
||||
} else {
|
||||
flash_range_program(flash_addr, data, len);
|
||||
}
|
||||
} else {
|
||||
/* Partial page at the start and/or end: read the page
|
||||
* back from XIP, merge in the write, program the whole
|
||||
* page. flash_range_program() only accepts page aligned
|
||||
* addresses and page multiple lengths. The AND program
|
||||
* keeps the trailer flag accumulation intact. */
|
||||
while (written < (uint32_t)len) {
|
||||
addr = flash_addr + written;
|
||||
page_off = addr & (FLASH_PAGE_SIZE - 1);
|
||||
page_addr = addr & ~(FLASH_PAGE_SIZE - 1);
|
||||
remaining = (uint32_t)len - written;
|
||||
|
||||
sz = FLASH_PAGE_SIZE - page_off;
|
||||
if (sz > remaining)
|
||||
sz = remaining;
|
||||
|
||||
memcpy(cache, (const uint8_t *)(XIP_BASE + page_addr),
|
||||
FLASH_PAGE_SIZE);
|
||||
memcpy(cache + page_off, data + written, sz);
|
||||
flash_range_program(page_addr, cache, FLASH_PAGE_SIZE);
|
||||
written += sz;
|
||||
}
|
||||
}
|
||||
} else
|
||||
flash_range_program(address - XIP_BASE, data, len);
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -117,6 +117,7 @@ TESTS+=unit-t10xx-flash-status
|
|||
TESTS+=unit-p1021-erase-advance
|
||||
TESTS+=unit-samr21-erase-advance
|
||||
TESTS+=unit-hifive1-flash-write
|
||||
TESTS+=unit-rp2350-flash-write
|
||||
TESTS+=unit-fwtpm-rsp-overrun
|
||||
TESTS+=unit-fwtpm-cmd-toctou
|
||||
TESTS+=unit-fdt-memrsv-wrap
|
||||
|
|
@ -1230,6 +1231,16 @@ hifive1_flash_write_extract.h: ../../hal/hifive1.c
|
|||
unit-hifive1-flash-write: unit-hifive1-flash-write.c hifive1_flash_write_extract.h
|
||||
gcc -o $@ unit-hifive1-flash-write.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-rp2350-flash-write runs the real hal_flash_write() from
|
||||
# hal/rp2350.c against a mock flash_range_program() that enforces the
|
||||
# ROM contract (F-12061: unaligned / non page-multiple writes from
|
||||
# the partition-state path violate the 256-byte page requirement).
|
||||
rp2350_flash_write_extract.h: ../../hal/rp2350.c
|
||||
sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< > $@
|
||||
|
||||
unit-rp2350-flash-write: unit-rp2350-flash-write.c rp2350_flash_write_extract.h
|
||||
gcc -o $@ unit-rp2350-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
|
||||
|
|
@ -1464,7 +1475,8 @@ covclean:
|
|||
GENERATED_SRC:=aurix_erased_extract.h \
|
||||
hifive1_flash_write_extract.h nvm_cache_scrub_extract.h \
|
||||
nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \
|
||||
p1021_erase_extract.h p1021_erase_fn_extract.h sdhci_host.c \
|
||||
p1021_erase_extract.h p1021_erase_fn_extract.h rp2350_flash_write_extract.h \
|
||||
sdhci_host.c \
|
||||
stm32g4_write_extract.h stm32l5_write_extract.h \
|
||||
stm32u5_write_extract.h \
|
||||
t10xx_flash_status_extract.h t10xx_qe_firmware_extract.h \
|
||||
|
|
|
|||
|
|
@ -0,0 +1,277 @@
|
|||
/* unit-rp2350-flash-write.c
|
||||
*
|
||||
* Regression test for F-12061: hal_flash_write() in hal/rp2350.c
|
||||
* chunked the write by WOLFBOOT_SECTOR_SIZE (8 KiB) and passed the
|
||||
* resulting sizes straight to pico-sdk flash_range_program(), which
|
||||
* requires a 256-byte page aligned address and a page multiple
|
||||
* length (invalid_params_if on both, no partial-page support). The
|
||||
* partition-state path without NVM_FLASH_WRITEONCE violates the
|
||||
* contract on every state transition: trailer_write() issues a
|
||||
* 1-byte write and partition_magic_write() a 4-byte write.
|
||||
*
|
||||
* The real function is extracted by the Makefile and run against a
|
||||
* mock flash_range_program() that enforces the ROM contract (any
|
||||
* unaligned or non page-multiple call is recorded as a violation)
|
||||
* and programs with flash AND semantics. The flash image is mmap'd
|
||||
* at XIP_BASE so the read-modify-write page reads hit real memory,
|
||||
* exactly as the XIP mapping would on hardware.
|
||||
*
|
||||
* 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
|
||||
|
||||
/* RP2350 XIP flash base (pico-sdk hardware/regs/addressmap.h). */
|
||||
#define XIP_BASE 0x10000000UL
|
||||
#define FLASH_PAGE_SIZE 256
|
||||
/* Sector size from config/examples/rp2350.config. */
|
||||
#define WOLFBOOT_SECTOR_SIZE 0x2000
|
||||
|
||||
#define FLASH_MEM_SZ (4 * FLASH_PAGE_SIZE)
|
||||
static uint8_t *g_flash; /* mmap'd at XIP_BASE */
|
||||
|
||||
/* "SRAM" source buffer: bit 29 set, so the HAL takes the in-RAM
|
||||
* direct-program branch, the same branch a stack/.bss pointer takes
|
||||
* on the real target (SRAM at 0x20000000). */
|
||||
#define SRAM_ADDR 0x21000000UL
|
||||
#define SRAM_SZ 1024
|
||||
static uint8_t *g_sram;
|
||||
|
||||
/* Contract violations recorded by the mock ROM call. */
|
||||
static int g_violations;
|
||||
|
||||
void flash_range_program(uint32_t flash_offs, const uint8_t *data,
|
||||
size_t count)
|
||||
{
|
||||
size_t i;
|
||||
|
||||
if ((flash_offs & (FLASH_PAGE_SIZE - 1)) ||
|
||||
(count & (FLASH_PAGE_SIZE - 1)) ||
|
||||
(flash_offs + count > FLASH_MEM_SZ)) {
|
||||
g_violations++;
|
||||
return;
|
||||
}
|
||||
/* Flash can only clear bits: programmed = old & new. */
|
||||
for (i = 0; i < count; i++)
|
||||
g_flash[flash_offs + i] &= data[i];
|
||||
}
|
||||
|
||||
/* The real hal_flash_write() from hal/rp2350.c (extracted). */
|
||||
#include "rp2350_flash_write_extract.h"
|
||||
|
||||
static void setup(void)
|
||||
{
|
||||
memset(g_flash, 0xFF, FLASH_MEM_SZ); /* erased */
|
||||
memset(g_sram, 0x5A, SRAM_SZ);
|
||||
g_violations = 0;
|
||||
}
|
||||
|
||||
static void teardown(void)
|
||||
{
|
||||
}
|
||||
|
||||
/* The exact partition-state write: trailer_write() without
|
||||
* NVM_FLASH_WRITEONCE compiles to hal_flash_write(addr, &val, 1).
|
||||
* Pre-fix the 1-byte length is passed to flash_range_program() and
|
||||
* violates the ROM contract; post-fix the page is RMW'd. */
|
||||
START_TEST(test_one_byte_trailer_write){
|
||||
uint8_t val = 0x7E;
|
||||
int i;
|
||||
|
||||
/* Seed the page with an existing pattern; only the write offset
|
||||
* is erased, so the programmed byte must land as requested while
|
||||
* every other byte of the page is preserved. */
|
||||
for (i = 0; i < FLASH_PAGE_SIZE; i++)
|
||||
g_flash[FLASH_PAGE_SIZE + i] = (i == 100) ? 0xFF :
|
||||
(uint8_t)(0xA0 ^ (i & 0x0F));
|
||||
g_sram[0] = val;
|
||||
|
||||
ck_assert_int_eq(hal_flash_write((uint32_t)(XIP_BASE + FLASH_PAGE_SIZE +
|
||||
100), g_sram, 1), 0);
|
||||
ck_assert_int_eq(g_violations, 0);
|
||||
ck_assert_uint_eq(g_flash[FLASH_PAGE_SIZE + 100], val);
|
||||
for (i = 0; i < FLASH_PAGE_SIZE; i++) {
|
||||
if (i != 100)
|
||||
ck_assert_uint_eq(g_flash[FLASH_PAGE_SIZE + i],
|
||||
(uint8_t)(0xA0 ^ (i & 0x0F)));
|
||||
}
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* partition_magic_write() compiles to a 4-byte write of the magic
|
||||
* trailer. Same contract violation pre-fix. */
|
||||
START_TEST(test_four_byte_magic_write)
|
||||
{
|
||||
uint32_t magic = 0x600DF00D;
|
||||
uint8_t *m = (uint8_t *)&magic;
|
||||
int i;
|
||||
|
||||
for (i = 0; i < FLASH_PAGE_SIZE; i++)
|
||||
g_flash[i] = (uint8_t)(0x0F ^ (i & 0x70));
|
||||
/* The magic region is erased, as on a fresh partition tail. */
|
||||
g_flash[200] = g_flash[201] = g_flash[202] = g_flash[203] = 0xFF;
|
||||
memcpy(g_sram, m, 4);
|
||||
|
||||
ck_assert_int_eq(hal_flash_write((uint32_t)(XIP_BASE + 200), g_sram, 4),
|
||||
0);
|
||||
ck_assert_int_eq(g_violations, 0);
|
||||
ck_assert_uint_eq(*(uint32_t *)(g_flash + 200), magic);
|
||||
for (i = 0; i < FLASH_PAGE_SIZE; i++) {
|
||||
if (i >= 200 && i <= 203)
|
||||
continue;
|
||||
ck_assert_uint_eq(g_flash[i], (uint8_t)(0x0F ^ (i & 0x70)));
|
||||
}
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* An unaligned write spanning three pages: pre-fix the whole 600
|
||||
* bytes go to flash_range_program() in one non-page-multiple call;
|
||||
* post-fix each page is RMW'd and the bytes outside the request are
|
||||
* preserved. */
|
||||
START_TEST(test_unaligned_multi_page_write)
|
||||
{
|
||||
int i;
|
||||
int ret;
|
||||
|
||||
for (i = 0; i < SRAM_SZ; i++)
|
||||
g_sram[i] = (uint8_t)(0x30 + (i & 0x0F));
|
||||
|
||||
ret = hal_flash_write((uint32_t)(XIP_BASE + 100), g_sram, 600);
|
||||
ck_assert_int_eq(ret, 0);
|
||||
ck_assert_int_eq(g_violations, 0);
|
||||
ck_assert_mem_eq(g_flash + 100, g_sram, 600);
|
||||
for (i = 0; i < 100; i++)
|
||||
ck_assert_uint_eq(g_flash[i], 0xFF);
|
||||
for (i = 700; i < FLASH_MEM_SZ; i++)
|
||||
ck_assert_uint_eq(g_flash[i], 0xFF);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* Page aligned start and page multiple length: the fast path. Data
|
||||
* in RAM is programmed directly, unchanged by the fix. */
|
||||
START_TEST(test_aligned_page_multiple_write_sram)
|
||||
{
|
||||
int i;
|
||||
|
||||
for (i = 0; i < 512; i++)
|
||||
g_sram[i] = (uint8_t)(0xC0 ^ (i & 0x1F));
|
||||
|
||||
ck_assert_int_eq(hal_flash_write((uint32_t)XIP_BASE, g_sram, 512), 0);
|
||||
ck_assert_int_eq(g_violations, 0);
|
||||
ck_assert_mem_eq(g_flash, g_sram, 512);
|
||||
for (i = 512; i < FLASH_MEM_SZ; i++)
|
||||
ck_assert_uint_eq(g_flash[i], 0xFF);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* Fast path with the source in XIP flash: the data must be staged
|
||||
* to RAM before programming (XIP is disabled while a page is
|
||||
* programmed). */
|
||||
START_TEST(test_aligned_page_multiple_write_xip)
|
||||
{
|
||||
int i;
|
||||
|
||||
for (i = 0; i < 512; i++)
|
||||
g_flash[3 * FLASH_PAGE_SIZE + i] = (uint8_t)(0x80 ^ (i & 0x3F));
|
||||
|
||||
ck_assert_int_eq(hal_flash_write((uint32_t)XIP_BASE,
|
||||
(uint8_t *)(XIP_BASE +
|
||||
3 * FLASH_PAGE_SIZE),
|
||||
512), 0);
|
||||
ck_assert_int_eq(g_violations, 0);
|
||||
for (i = 0; i < 512; i++)
|
||||
ck_assert_uint_eq(g_flash[i], (uint8_t)(0x80 ^ (i & 0x3F)));
|
||||
}
|
||||
END_TEST
|
||||
|
||||
/* Aligned start, non page-multiple length, source in XIP: pre-fix
|
||||
* the 300-byte tail is passed to flash_range_program() as-is;
|
||||
* post-fix the last partial page is RMW'd. Source (pages 0-1) and
|
||||
* destination (pages 2-3) do not overlap. */
|
||||
START_TEST(test_xip_partial_write)
|
||||
{
|
||||
int i;
|
||||
|
||||
for (i = 0; i < 300; i++)
|
||||
g_flash[i] = (uint8_t)(0x40 + (i & 0x07));
|
||||
|
||||
ck_assert_int_eq(hal_flash_write((uint32_t)(XIP_BASE + 2 * FLASH_PAGE_SIZE),
|
||||
(uint8_t *)XIP_BASE,
|
||||
300), 0);
|
||||
ck_assert_int_eq(g_violations, 0);
|
||||
ck_assert_mem_eq(g_flash + 2 * FLASH_PAGE_SIZE, g_flash, 300);
|
||||
/* Source region untouched. */
|
||||
for (i = 0; i < 300; i++)
|
||||
ck_assert_uint_eq(g_flash[i], (uint8_t)(0x40 + (i & 0x07)));
|
||||
for (i = 300; i < 2 * FLASH_PAGE_SIZE; i++)
|
||||
ck_assert_uint_eq(g_flash[i], 0xFF);
|
||||
for (i = 2 * FLASH_PAGE_SIZE + 300; i < FLASH_MEM_SZ; i++)
|
||||
ck_assert_uint_eq(g_flash[i], 0xFF);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
Suite *rp2350_flash_write_suite(void)
|
||||
{
|
||||
Suite *s = suite_create("rp2350 flash write");
|
||||
TCase *tc = tcase_create("page-contract");
|
||||
|
||||
tcase_add_checked_fixture(tc, setup, teardown);
|
||||
tcase_add_test(tc, test_one_byte_trailer_write);
|
||||
tcase_add_test(tc, test_four_byte_magic_write);
|
||||
tcase_add_test(tc, test_unaligned_multi_page_write);
|
||||
tcase_add_test(tc, test_aligned_page_multiple_write_sram);
|
||||
tcase_add_test(tc, test_aligned_page_multiple_write_xip);
|
||||
tcase_add_test(tc, test_xip_partial_write);
|
||||
tcase_set_timeout(tc, 10);
|
||||
suite_add_tcase(s, tc);
|
||||
return s;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int fails;
|
||||
Suite *s = rp2350_flash_write_suite();
|
||||
SRunner *sr = srunner_create(s);
|
||||
|
||||
g_flash = mmap((void *)XIP_BASE, FLASH_MEM_SZ,
|
||||
PROT_READ | PROT_WRITE,
|
||||
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
|
||||
if (g_flash == MAP_FAILED)
|
||||
return 99;
|
||||
g_sram = mmap((void *)SRAM_ADDR, SRAM_SZ,
|
||||
PROT_READ | PROT_WRITE,
|
||||
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
|
||||
if (g_sram == MAP_FAILED)
|
||||
return 99;
|
||||
|
||||
srunner_run_all(sr, CK_NORMAL);
|
||||
fails = srunner_ntests_failed(sr);
|
||||
srunner_free(sr);
|
||||
|
||||
munmap(g_flash, FLASH_MEM_SZ);
|
||||
munmap(g_sram, SRAM_SZ);
|
||||
|
||||
return fails;
|
||||
}
|
||||
Loading…
Reference in New Issue