mirror of https://github.com/wolfSSL/wolfBoot.git
F-7380: make SAMA5D3 ext_flash_read byte-accurate for partial pages
ext_flash_read() ignored the intra-page offset of the start address (a partial read returned bytes from the head of the page), copied sub-page reads in 32-bit words (dropping a sub-word tail), and wrote a full NAND page into the caller's buffer when a multi-page read ended mid-page (overrunning it). The integrity check hashes the image in 64-byte blocks from fw_base + offset, so any read crossing a page mid-block returned the wrong bytes. Walk the read page by page from the exact address: full aligned pages go straight into the caller's buffer; partial first/last pages are staged through the page buffer and copied from the column offset. Proven by unit-sama5d3-ext-read, which extracts the real function and runs it against an emulated device: unaligned starts, 1-3 byte lengths, the 64-byte integrity-hash block pattern, page/block boundaries, and multi-page reads with a short tail. Pre-fix 6 of the 8 checks failed.pull/868/head
Daniele Lacamera
Daniele Lacamera
parent
5c07c3797e
commit
f5401b0e23
|
|
@ -478,12 +478,14 @@ tools/unit-tests/unit-versal-qspi-dma
|
|||
tools/unit-tests/unit-xspi-tfd-index
|
||||
tools/unit-tests/unit-zynq-erase-loop
|
||||
tools/unit-tests/unit-zynq-ext-write
|
||||
tools/unit-tests/unit-sama5d3-ext-read
|
||||
# sources generated by the unit-test extraction rules
|
||||
tools/unit-tests/aurix_erased_extract.h
|
||||
tools/unit-tests/nvm_cache_scrub_extract.h
|
||||
tools/unit-tests/nxp_ls1028a_host.c
|
||||
tools/unit-tests/nxp_p1021_host.c
|
||||
tools/unit-tests/nxp_t10xx_fixup_extract.h
|
||||
tools/unit-tests/sama5d3_read_extract.h
|
||||
tools/unit-tests/sdhci_host.c
|
||||
tools/unit-tests/stm32l5_write_extract.h
|
||||
tools/unit-tests/stm32u5_write_extract.h
|
||||
|
|
|
|||
|
|
@ -601,28 +601,29 @@ static int nand_check_bad_block(uint32_t block)
|
|||
int ext_flash_read(uintptr_t address, uint8_t *data, int len)
|
||||
{
|
||||
uint8_t buffer_page[NAND_FLASH_PAGE_SIZE];
|
||||
uint32_t block = div_u(address, nand_flash.block_size); /* The block where the address falls in */
|
||||
uint32_t page = div_u(address, nand_flash.page_size); /* The page where the address falls in */
|
||||
uint32_t start_page_in_block = mod(page, nand_flash.pages_per_block); /* The start page within this block */
|
||||
uint32_t in_block_offset = mod(address, nand_flash.block_size); /* The offset of the address within the block */
|
||||
uint32_t remaining = nand_flash.block_size - in_block_offset; /* How many bytes remaining to read in the first block */
|
||||
int len_to_read = len;
|
||||
uint8_t *buffer = data;
|
||||
uint32_t i;
|
||||
int copy = 0;
|
||||
uintptr_t addr = address;
|
||||
uint8_t *dst = data;
|
||||
uint32_t in_page = mod(address, nand_flash.page_size); /* The offset of the address within the page */
|
||||
int remaining = len;
|
||||
int ret;
|
||||
|
||||
if (len < (int)nand_flash.page_size) {
|
||||
buffer = buffer_page;
|
||||
copy = 1;
|
||||
len_to_read = nand_flash.page_size;
|
||||
}
|
||||
if (len <= 0)
|
||||
return 0;
|
||||
|
||||
while (len_to_read > 0) {
|
||||
uint32_t sz = len_to_read;
|
||||
uint32_t pages_to_read;
|
||||
if (sz > remaining)
|
||||
sz = remaining;
|
||||
while (remaining > 0) {
|
||||
uint32_t block;
|
||||
uint32_t page;
|
||||
uint32_t page_in_block;
|
||||
uint32_t chunk;
|
||||
|
||||
/* Bytes available from the current address to the end of its page */
|
||||
chunk = nand_flash.page_size - in_page;
|
||||
if (chunk > (uint32_t)remaining)
|
||||
chunk = (uint32_t)remaining;
|
||||
|
||||
block = div_u(addr, nand_flash.block_size);
|
||||
page = div_u(addr, nand_flash.page_size);
|
||||
page_in_block = mod(page, nand_flash.pages_per_block);
|
||||
|
||||
do {
|
||||
ret = nand_check_bad_block(block);
|
||||
|
|
@ -632,32 +633,23 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len)
|
|||
}
|
||||
} while (ret < 0);
|
||||
|
||||
/* Amount of pages to be read from this block */
|
||||
pages_to_read = div_u((sz + nand_flash.page_size - 1), nand_flash.page_size);
|
||||
|
||||
if (pages_to_read * nand_flash.page_size > remaining)
|
||||
pages_to_read--;
|
||||
|
||||
/* Read (remaining) pages off a block */
|
||||
for (i = 0; i < pages_to_read; i++) {
|
||||
nand_read_page(block, start_page_in_block + i, buffer);
|
||||
if (sz > nand_flash.page_size)
|
||||
sz = nand_flash.page_size;
|
||||
len_to_read -= sz;
|
||||
buffer += sz;
|
||||
if ((in_page == 0) && (chunk == nand_flash.page_size)) {
|
||||
/* Full page at the page head: read straight into the caller's
|
||||
* buffer. */
|
||||
nand_read_page(block, page_in_block, dst);
|
||||
}
|
||||
/* The block is over, move to the next one */
|
||||
block++;
|
||||
start_page_in_block = 0;
|
||||
remaining = nand_flash.block_size;
|
||||
}
|
||||
if (copy) {
|
||||
uint32_t *dst = (uint32_t *)data;
|
||||
uint32_t *src = (uint32_t *)buffer_page;
|
||||
uint32_t tot_len = (uint32_t)len;
|
||||
for (i = 0; i < (tot_len >> 2); i++) {
|
||||
dst[i] = src[i];
|
||||
else {
|
||||
/* Partial first/last page: stage through the page buffer and
|
||||
* copy from the column offset, so a full page is never written
|
||||
* past the end of the caller's buffer. */
|
||||
nand_read_page(block, page_in_block, buffer_page);
|
||||
memcpy(dst, buffer_page + in_page, chunk);
|
||||
}
|
||||
|
||||
dst += chunk;
|
||||
remaining -= (int)chunk;
|
||||
addr += chunk;
|
||||
in_page = mod(addr, nand_flash.page_size);
|
||||
}
|
||||
return len;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -150,6 +150,7 @@ ENABLE_32BIT_TESTS ?= $(HAVE_M32)
|
|||
ifeq ($(ENABLE_32BIT_TESTS),1)
|
||||
TESTS+=unit-linux-loader-e820
|
||||
TESTS+=unit-linux-loader-syssize
|
||||
TESTS+=unit-sama5d3-ext-read
|
||||
else
|
||||
$(info Skipping 32-bit x86 linux-loader unit tests: 'gcc -m32' unavailable (set ENABLE_32BIT_TESTS=1 to force))
|
||||
endif
|
||||
|
|
@ -837,6 +838,20 @@ zynq_write_extract.h: ../../hal/zynq.c
|
|||
unit-zynq-ext-write: unit-zynq-ext-write.c zynq_write_extract.h
|
||||
gcc -o $@ unit-zynq-ext-write.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-sama5d3-ext-read runs the real ext_flash_read() from hal/sama5d3.c
|
||||
# (the intra-page offset of the start address was never applied, sub-page
|
||||
# reads were copied in 32-bit words, and a multi-page read ending
|
||||
# mid-page wrote a full page past the caller's buffer). The function and
|
||||
# the nand_flash geometry struct are extracted verbatim; the test provides
|
||||
# host div_u()/mod() and emulated nand_read_page()/nand_check_bad_block()
|
||||
# backed by a byte array.
|
||||
sama5d3_read_extract.h: ../../hal/sama5d3.c
|
||||
sed -n '/^struct nand_flash {/,/^} nand_flash = { 0 };/p' $< > $@
|
||||
sed -n '/^int ext_flash_read(/,/^}/p' $< >> $@
|
||||
|
||||
unit-sama5d3-ext-read: unit-sama5d3-ext-read.c sama5d3_read_extract.h
|
||||
gcc -o $@ unit-sama5d3-ext-read.c $(CFLAGS) $(LDFLAGS)
|
||||
|
||||
# unit-versal-qspi-dma drives the real DMA RX path of qspi_transfer() in
|
||||
# hal/versal.c (an unaligned read larger than the 4096-byte
|
||||
# temp buffer copied the full requested length out of the buffer). The
|
||||
|
|
|
|||
|
|
@ -0,0 +1,244 @@
|
|||
/* unit-sama5d3-ext-read.c
|
||||
*
|
||||
* Regression test: ext_flash_read() in hal/sama5d3.c never applied the
|
||||
* intra-page offset of the start address (a partial read returned bytes
|
||||
* from the head of the page instead of the requested column), copied
|
||||
* sub-page reads in 32-bit words (dropping a sub-word tail), and wrote
|
||||
* a full NAND page into the caller's buffer for a multi-page read
|
||||
* ending mid-page (overrunning the buffer).
|
||||
*
|
||||
* The HAL cannot be built on the host, so the Makefile extracts
|
||||
* ext_flash_read() verbatim along with the nand_flash geometry struct;
|
||||
* the test provides host div_u()/mod() (the HAL's software-division
|
||||
* wrappers exist only because the Cortex-A5 has no divider) and
|
||||
* emulated nand_read_page()/nand_check_bad_block() backed by a
|
||||
* deterministic byte array.
|
||||
* 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 <stdio.h>
|
||||
#include <string.h>
|
||||
|
||||
/* sama5d3.h constants the extracted function needs. */
|
||||
#define NAND_FLASH_PAGE_SIZE 0x800 /* 2KB */
|
||||
#define NAND_FLASH_OOB_SIZE 0x40 /* 64B */
|
||||
#define MAX_ECC_BYTES 8
|
||||
#define wolfBoot_printf(...) do {} while (0)
|
||||
|
||||
/* Emulated NAND: 8 blocks x 16 pages x 2KB = 256KB. */
|
||||
#define EMU_BLOCKS 8
|
||||
#define EMU_PAGES_PER_BLK 16
|
||||
#define EMU_PAGE_SIZE NAND_FLASH_PAGE_SIZE
|
||||
#define EMU_BLOCK_SIZE (EMU_PAGES_PER_BLK * EMU_PAGE_SIZE)
|
||||
#define EMU_TOTAL (EMU_BLOCKS * EMU_BLOCK_SIZE)
|
||||
static uint8_t emu_nand[EMU_TOTAL];
|
||||
|
||||
/* Host equivalents of the HAL's software-division wrappers. */
|
||||
static uint32_t div_u(uint32_t dividend, uint32_t divisor)
|
||||
{
|
||||
return dividend / divisor;
|
||||
}
|
||||
|
||||
static uint32_t mod(uint32_t dividend, uint32_t divisor)
|
||||
{
|
||||
return dividend % divisor;
|
||||
}
|
||||
|
||||
/* Emulated NAND primitives: a full page read from column 0, like the
|
||||
* hardware path; every block is good. */
|
||||
static int nand_check_bad_block(uint32_t block)
|
||||
{
|
||||
(void)block;
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int nand_read_page(uint32_t block, uint32_t page, uint8_t *data)
|
||||
{
|
||||
uint32_t row = block * EMU_PAGES_PER_BLK + page;
|
||||
|
||||
memcpy(data, emu_nand + row * EMU_PAGE_SIZE, EMU_PAGE_SIZE);
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* The real ext_flash_read() and nand_flash struct from hal/sama5d3.c
|
||||
* (extracted by the Makefile). */
|
||||
#include "sama5d3_read_extract.h"
|
||||
|
||||
/* Read buffer plus an adjacent canary region: a multi-page read ending
|
||||
* mid-page used to write a full page past the requested length. */
|
||||
#define SCRATCH_LEN (EMU_PAGE_SIZE * 4)
|
||||
static uint8_t scratch[SCRATCH_LEN + 64];
|
||||
#define CANARY (scratch + SCRATCH_LEN)
|
||||
#define CANARY_LEN 64
|
||||
|
||||
static void setup(void)
|
||||
{
|
||||
uint32_t i;
|
||||
|
||||
/* Deterministic pattern: every byte depends on row and column. */
|
||||
for (i = 0; i < EMU_TOTAL; i++)
|
||||
emu_nand[i] = (uint8_t)((i / EMU_PAGE_SIZE) * 7 + i);
|
||||
|
||||
nand_flash.page_size = EMU_PAGE_SIZE;
|
||||
nand_flash.block_size = EMU_BLOCK_SIZE;
|
||||
nand_flash.block_count = EMU_BLOCKS;
|
||||
nand_flash.pages_per_block = EMU_PAGES_PER_BLK;
|
||||
nand_flash.pages_per_device = EMU_BLOCKS * EMU_PAGES_PER_BLK;
|
||||
nand_flash.total_size = EMU_TOTAL;
|
||||
}
|
||||
|
||||
static void teardown(void)
|
||||
{
|
||||
}
|
||||
|
||||
static void fill_expected(uint8_t *dst, uint32_t address, uint32_t len)
|
||||
{
|
||||
uint32_t i;
|
||||
|
||||
for (i = 0; i < len; i++)
|
||||
dst[i] = emu_nand[address + i];
|
||||
}
|
||||
|
||||
/* Run one read and compare against the emulated device byte for byte. */
|
||||
static void read_case(uint32_t address, int len)
|
||||
{
|
||||
uint8_t expected[SCRATCH_LEN];
|
||||
int i;
|
||||
int ret;
|
||||
|
||||
ck_assert_int_lt(len, SCRATCH_LEN);
|
||||
memset(scratch, 0xEE, sizeof(scratch));
|
||||
fill_expected(expected, address, (uint32_t)len);
|
||||
|
||||
ret = ext_flash_read(address, scratch, len);
|
||||
ck_assert_int_eq(ret, len);
|
||||
if (len > 0)
|
||||
ck_assert_mem_eq(scratch, expected, (size_t)len);
|
||||
|
||||
/* The canary must be untouched: nothing may be written past len. */
|
||||
for (i = 0; i < CANARY_LEN; i++)
|
||||
ck_assert_uint_eq(CANARY[i], 0xEE);
|
||||
}
|
||||
|
||||
START_TEST(test_read_zero_length)
|
||||
{
|
||||
memset(scratch, 0xEE, sizeof(scratch));
|
||||
ck_assert_int_eq(ext_flash_read(0x4000, scratch, 0), 0);
|
||||
ck_assert_uint_eq(scratch[0], 0xEE);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_aligned_full_page)
|
||||
{
|
||||
read_case(0, EMU_PAGE_SIZE);
|
||||
read_case(0x10000, EMU_PAGE_SIZE);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_unaligned_small)
|
||||
{
|
||||
/* Mid-page starts: the head of the page must not be returned. */
|
||||
read_case(1, 4);
|
||||
read_case(0x7FC, 8);
|
||||
read_case(0x400, 64);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_subword_lengths)
|
||||
{
|
||||
/* 1-3 byte reads copy nothing in a word-count loop. */
|
||||
read_case(0x800, 1);
|
||||
read_case(0x801, 2);
|
||||
read_case(0x1800, 3);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_sha_block_pattern)
|
||||
{
|
||||
/* The integrity check hashes the image in 64-byte blocks from
|
||||
* fw_base + offset: every block after the first in each page is
|
||||
* an unaligned small read. */
|
||||
uint32_t offset;
|
||||
|
||||
for (offset = 0; offset < EMU_PAGE_SIZE; offset += 0x40)
|
||||
read_case(0x800 + offset, 64);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_page_boundaries)
|
||||
{
|
||||
read_case(0, EMU_PAGE_SIZE - 1);
|
||||
read_case(0, EMU_PAGE_SIZE + 1);
|
||||
/* Start near the end of a page and cross into the next. */
|
||||
read_case(0x7F0, 0x20);
|
||||
read_case(0x7FF, 0x101);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_multipage_partial_tail)
|
||||
{
|
||||
/* Multi-page reads whose tail is shorter than a page (and not a
|
||||
* multiple of 4): the tail page must not be written in full. */
|
||||
read_case(0x100, 0x903);
|
||||
read_case(0, 0x1805);
|
||||
read_case(0x40, 0x1F01);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_read_cross_block)
|
||||
{
|
||||
read_case(0x7F00, 0x120);
|
||||
read_case(0x7000, 0x200);
|
||||
read_case(0x7001, 0x1000);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
Suite *sama5d3_ext_read_suite(void)
|
||||
{
|
||||
Suite *s = suite_create("sama5d3-ext-read");
|
||||
TCase *tc = tcase_create("sama5d3-ext-read");
|
||||
|
||||
tcase_add_checked_fixture(tc, setup, teardown);
|
||||
tcase_add_test(tc, test_read_zero_length);
|
||||
tcase_add_test(tc, test_read_aligned_full_page);
|
||||
tcase_add_test(tc, test_read_unaligned_small);
|
||||
tcase_add_test(tc, test_read_subword_lengths);
|
||||
tcase_add_test(tc, test_read_sha_block_pattern);
|
||||
tcase_add_test(tc, test_read_page_boundaries);
|
||||
tcase_add_test(tc, test_read_multipage_partial_tail);
|
||||
tcase_add_test(tc, test_read_cross_block);
|
||||
suite_add_tcase(s, tc);
|
||||
|
||||
return s;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int fails;
|
||||
Suite *s = sama5d3_ext_read_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