WolfSSL
/
wolfBoot
mirror of https://github.com/wolfSSL/wolfBoot.git
1
0
Fork 0
Code Issues Releases Wiki Activity

F-12064: p1021: drop already-delivered pages of a bad block 

When the bad-block marker is found on the block's second page, the
first page has already been copied to the caller's buffer and counted
in pos; the skip advanced only the source address, so the bad block's
page stayed in the output and the read returned len with the bad
block's content mixed into the image.

Record the output position at the start of each erase block and, on a
bad block, rewind both pos and the data pointer to it before advancing
the source address. This preserves the data = original + pos
invariant (no out-of-bounds write) and discards the bad block's
delivered pages.

test_bad_marker_second_page_dropped now asserts the bad block is fully
skipped (output starts at the next block's first page); it fails
against the previous code.
pull/880/head
Daniele Lacamera 2026-09-02 13:17:48 +02:00
parent a504552883
commit fa0c329a02
2 changed files with 38 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
hal/nxp_p1021.c
View File

@ -1706,6 +1706,8 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len)
uint32_t block_size, page_size, read_size;
int ret = 0, pos = 0, i = 0;
int bad_marker;
uint8_t *block_start_data;
int block_start_pos;
#ifdef DEBUG_EXT_FLASH
wolfBoot_printf("ext read: addr 0x%x, dst 0x%x, len %d\n",
@ -1739,8 +1741,13 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len)
/* total download loop */
while (pos < len) {
/* the bad-block marker only exists on the first pages of each
* erase block: restart the per-block page counter */
* erase block: restart the per-block page counter. Record the
* output position at the start of the block so that, if the
* block turns out to be bad, the pages already copied from it
* can be discarded. */
i = 0;
block_start_data = data;
block_start_pos = pos;
/* block loop */
do {
@ -1769,10 +1776,14 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len)
/* check for bad page. if either of the first two pages are bad then
* skip to next block */
if (i++ < 2 && flash_buf[bad_marker] != 0xFF) {
/* skip block: the bad block's bytes are not delivered
* and the read continues at the next block. pos and
* data already agree (data = original + pos), so only
* the source address moves. */
/* bad block: discard the pages already copied from it
* (the marker is only checked on the first two pages, so
* a page may have been delivered before detection) and
* continue at the next block. Rewind pos and data to the
* block start (data = original + pos is preserved) and
* move the source address past the bad block. */
pos = block_start_pos;
data = block_start_data;
address = (address + block_size) & ~(block_size - 1);
break;
}

39
tools/unit-tests/unit-p1021-read-badblock.c
View File

@ -1,12 +1,15 @@
/* unit-p1021-read-badblock.c
*
* Regression test for F-12064: ext_flash_read() in hal/nxp_p1021.c
* kept its bad-block page counter for the whole request, so the
* marker was inspected only on the first two pages read and a bad
* block later in the request was copied as valid data. When a
* marker did cause a skip, the logical position was rewound to a
* block boundary while the output pointer was not, so the read
* continued past the end of the caller's buffer.
* Regression test for F-12064 (and the follow-up bad-block skip
* review): ext_flash_read() in hal/nxp_p1021.c kept its bad-block
* page counter for the whole request, so the marker was inspected
* only on the first two pages read and a bad block later in the
* request was copied as valid data. When a marker did cause a skip,
* the logical position was rewound to a block boundary while the
* output pointer was not, so the read continued past the end of the
* caller's buffer. A related defect kept the pages of a bad block
* that were already delivered before the marker was found on its
* second page; those pages must be discarded as well.
*
* The real function is extracted by the Makefile together with the
* ELBC register macros it uses; the ELBC register access and the
@ -157,12 +160,12 @@ START_TEST (test_bad_block_in_later_block_skipped){
}
END_TEST
/* Bad marker on the second page of the first block: one page was
* already delivered when the skip fires. Post-fix the output
* pointer and the position stay consistent and nothing is written
* past the buffer; pre-fix the position was rewound while the
* pointer was not, overflowing the buffer by one page. */
START_TEST(test_bad_marker_second_page_no_overflow)
/* Bad marker on the second page of the first block: page 0 was
* already delivered when the skip fires, so it must be discarded
* along with the rest of the bad block. The output is the first two
* pages of block 1, nothing is written past the buffer, and the
* return value is still the full requested length. */
START_TEST(test_bad_marker_second_page_dropped)
{
uint8_t out[1024 + 64];
int ret, i;
@ -175,9 +178,11 @@ START_TEST(test_bad_marker_second_page_no_overflow)
ret = ext_flash_read(0, out, 1024);
ck_assert_int_eq(ret, 1024);
/* page 0 of block 0, then block 1 from its first page */
ck_assert_int_eq(memcmp(out, g_nand[0], SIM_PAGE_SIZE), 0);
ck_assert_int_eq(memcmp(out + SIM_PAGE_SIZE, g_nand[SIM_BLOCK_PAGES],
/* block 0 fully skipped: output starts at block 1 page 0 */
ck_assert_int_eq(memcmp(out, g_nand[SIM_BLOCK_PAGES],
SIM_PAGE_SIZE), 0);
ck_assert_int_eq(memcmp(out + SIM_PAGE_SIZE,
g_nand[SIM_BLOCK_PAGES + 1],
SIM_PAGE_SIZE), 0);
for (i = 1024; i < (int)sizeof(out); i++)
ck_assert_uint_eq(out[i], 0xEE);
@ -253,7 +258,7 @@ Suite *p1021_read_badblock_suite(void)
TCase *tc = tcase_create("bad-block");
tcase_add_test(tc, test_bad_block_in_later_block_skipped);
tcase_add_test(tc, test_bad_marker_second_page_no_overflow);
tcase_add_test(tc, test_bad_marker_second_page_dropped);
tcase_add_test(tc, test_bad_first_block_page0);
tcase_add_test(tc, test_all_good_two_blocks);
tcase_add_test(tc, test_unaligned_start_across_pages);