diff --git a/hal/cc26x2.c b/hal/cc26x2.c index eb8eb13606..0eb8502ce1 100644 --- a/hal/cc26x2.c +++ b/hal/cc26x2.c @@ -45,7 +45,10 @@ int uart_read_nonblock(char *c) int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) { - FlashProgram(data, address, len); + /* driverlib's FlashProgram() takes a non-const buffer but only reads + * from it. */ + if (FlashProgram((uint8_t *)data, address, len) != FAPI_STATUS_SUCCESS) + return -1; while(FlashCheckFsmForReady() != FAPI_STATUS_FSM_READY) ; return 0; @@ -64,7 +67,9 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) { int i = 0; while (len > 0) { - FlashSectorErase(address + (WOLFBOOT_SECTOR_SIZE * i++)); + if (FlashSectorErase(address + (WOLFBOOT_SECTOR_SIZE * i++)) != + FAPI_STATUS_SUCCESS) + return -1; while(FlashCheckFsmForReady() != FAPI_STATUS_FSM_READY) ; diff --git a/hal/stm32n6.c b/hal/stm32n6.c index 5f2f0efad8..d67a8a3d60 100644 --- a/hal/stm32n6.c +++ b/hal/stm32n6.c @@ -138,20 +138,28 @@ static int RAMFUNCTION octospi_cmd(uint8_t fmode, uint8_t cmd, return -1; } -static void RAMFUNCTION octospi_write_enable(void) +static int RAMFUNCTION octospi_write_enable(void) { - octospi_cmd(0, WRITE_ENABLE_CMD, 0, SPI_MODE_NONE, - NULL, 0, SPI_MODE_NONE, 0); + /* A failed WREN leaves the write-enable latch clear: the device then + * silently ignores the program/erase that follows, never goes BUSY, and + * octospi_wait_ready() reports idle on its first poll. The command must + * fail the operation instead of being discarded. */ + return octospi_cmd(0, WRITE_ENABLE_CMD, 0, SPI_MODE_NONE, + NULL, 0, SPI_MODE_NONE, 0); } -static void RAMFUNCTION octospi_wait_ready(void) +static int RAMFUNCTION octospi_wait_ready(void) { uint8_t sr; do { sr = 0; - octospi_cmd(1, READ_SR_CMD, 0, SPI_MODE_NONE, - &sr, 1, SPI_MODE_SINGLE, 0); + /* A failed status-register transfer must not read as "ready": + * sr stays zero and the loop would exit as if the flash were idle. */ + if (octospi_cmd(1, READ_SR_CMD, 0, SPI_MODE_NONE, + &sr, 1, SPI_MODE_SINGLE, 0) < 0) + return -1; } while (sr & FLASH_SR_BUSY); + return 0; } static void RAMFUNCTION octospi_enable_mmap(void) @@ -730,14 +738,20 @@ static int RAMFUNCTION nor_flash_write(uint32_t offset, const uint8_t *data, memcpy(page_buf, data, write_sz); - octospi_write_enable(); + if (octospi_write_enable() < 0) { + ret = -1; + break; + } ret = octospi_cmd(0, PAGE_PROG_4B_CMD, offset, SPI_MODE_SINGLE, page_buf, write_sz, SPI_MODE_SINGLE, 0); if (ret < 0) break; - octospi_wait_ready(); + if (octospi_wait_ready() < 0) { + ret = -1; + break; + } offset += write_sz; data += write_sz; @@ -759,14 +773,20 @@ static int RAMFUNCTION nor_flash_erase(uint32_t offset, int len) end = offset + len; while (offset < end) { - octospi_write_enable(); + if (octospi_write_enable() < 0) { + ret = -1; + break; + } ret = octospi_cmd(0, SEC_ERASE_4B_CMD, offset, SPI_MODE_SINGLE, NULL, 0, SPI_MODE_NONE, 0); if (ret < 0) break; - octospi_wait_ready(); + if (octospi_wait_ready() < 0) { + ret = -1; + break; + } offset += FLASH_SECTOR_SIZE; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 5bc64e381a..fa9735cbb6 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -100,6 +100,7 @@ TESTS+=unit-arm-tee-psa-ipc TESTS+=unit-dice-token-size TESTS+=unit-dice-token-nosign TESTS+=unit-va416x0-fram +TESTS+=unit-flash-write-cc26x2 TESTS+=unit-flash-write-mcxa TESTS+=unit-flash-write-nrf52 TESTS+=unit-flash-write-samr21 @@ -749,6 +750,12 @@ unit-ata-security-passphrase-zeroize: ../../include/target.h unit-ata-security-p gcc -o $@ unit-ata-security-passphrase-zeroize.c $(CFLAGS) \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +# unit-flash-write-cc26x2 includes hal/cc26x2.c directly, with cc26x2_ti_stub/ +# standing in for the (not vendored) TI CC26x2 SDK headers it includes. This +# is also the only build coverage hal/cc26x2.c has. +unit-flash-write-cc26x2: ../../include/target.h unit-flash-write-cc26x2.c ../../hal/cc26x2.c + gcc -o $@ unit-flash-write-cc26x2.c -Icc26x2_ti_stub $(CFLAGS) $(LDFLAGS) + # unit-flash-write-mcxa includes hal/mcxa.c directly, with mcxa_fsl_stub/ # standing in for the (not vendored) NXP MCUXpresso SDK headers it includes. unit-flash-write-mcxa: unit-flash-write-mcxa.c ../../hal/mcxa.c diff --git a/tools/unit-tests/cc26x2_ti_stub/oscillators.h b/tools/unit-tests/cc26x2_ti_stub/oscillators.h new file mode 100644 index 0000000000..852ad02397 --- /dev/null +++ b/tools/unit-tests/cc26x2_ti_stub/oscillators.h @@ -0,0 +1,7 @@ +/* Stand-in for the CC26x2 SDK/board oscillators.h that hal/cc26x2.c includes. + * Nothing from it is used by the flash paths under test; the real header + * only declares the oscillator setup that clock_init() performs. */ +#ifndef CC26X2_OSCILLATORS_STUB_H +#define CC26X2_OSCILLATORS_STUB_H + +#endif /* CC26X2_OSCILLATORS_STUB_H */ diff --git a/tools/unit-tests/cc26x2_ti_stub/ti-lib.h b/tools/unit-tests/cc26x2_ti_stub/ti-lib.h new file mode 100644 index 0000000000..e0f05b467f --- /dev/null +++ b/tools/unit-tests/cc26x2_ti_stub/ti-lib.h @@ -0,0 +1,73 @@ +/* Minimal stand-in for the TI CC26x2 driverlib wrappers (ti-lib.h) that + * hal/cc26x2.c includes. Only what that file references is declared here: + * the Fapi flash API used by hal_flash_write()/hal_flash_erase() (the unit + * test provides those bodies), plus the UART and PRCM/VIMS entry points used + * by uart_read()/hal_init(). The latter are not exercised by the test -- they + * only have to compile and link, so they are no-op inlines here. */ +#ifndef CC26X2_TI_LIB_STUB_H +#define CC26X2_TI_LIB_STUB_H + +#include +#include + +/* Fapi status codes (mirror driverlib/flash.h) */ +#define FAPI_STATUS_SUCCESS 0x00000000UL +#define FAPI_STATUS_FSM_BUSY 0x00000001UL +#define FAPI_STATUS_FSM_READY 0x00000002UL +#define FAPI_STATUS_FSM_ERROR 0x00000003UL + +/* Flash API. Definitions live in the unit test. The data pointer is + * non-const, matching driverlib/flash.h. */ +uint32_t FlashProgram(uint8_t *pui8DataBuffer, uint32_t ui32Address, + uint32_t ui32Count); +uint32_t FlashSectorErase(uint32_t ui32SectorAddress); +uint32_t FlashCheckFsmForReady(void); + +/* UART. Definitions live in the unit test. */ +#define UART0_BASE 0x40001000UL +int32_t UARTCharGet(uint32_t ui32Base); +int32_t UARTCharGetNonBlocking(uint32_t ui32Base); + +/* PRCM / VIMS constants and no-op entry points used by hal_init() */ +#define VIMS_BASE 0x40034000UL +#define VIMS_MODE_ENABLED 0x00000000UL +#define PRCM_DOMAIN_PERIPH 0x00000004UL +#define PRCM_DOMAIN_SERIAL 0x00000002UL +#define PRCM_DOMAIN_POWER_ON 0x00000001UL +#define PRCM_PERIPH_GPIO 0x00000010UL +#define PRCM_PERIPH_UART0 0x00000200UL + +static inline void ti_lib_vims_mode_set(uint32_t base, uint32_t mode) +{ + (void)base; (void)mode; +} + +static inline void ti_lib_vims_configure(uint32_t base, bool round_robin, + bool prefetch) +{ + (void)base; (void)round_robin; (void)prefetch; +} + +static inline void ti_lib_int_master_disable(void) { } +static inline void ti_lib_int_master_enable(void) { } + +static inline void ti_lib_prcm_power_domain_on(uint32_t domain) +{ + (void)domain; +} + +static inline uint32_t ti_lib_prcm_power_domain_status(uint32_t domain) +{ + (void)domain; + return PRCM_DOMAIN_POWER_ON; +} + +static inline void ti_lib_prcm_peripheral_run_enable(uint32_t periph) +{ + (void)periph; +} + +static inline void ti_lib_prcm_load_set(void) { } +static inline bool ti_lib_prcm_load_get(void) { return true; } + +#endif /* CC26X2_TI_LIB_STUB_H */ diff --git a/tools/unit-tests/unit-flash-write-cc26x2.c b/tools/unit-tests/unit-flash-write-cc26x2.c new file mode 100644 index 0000000000..8eca198f43 --- /dev/null +++ b/tools/unit-tests/unit-flash-write-cc26x2.c @@ -0,0 +1,222 @@ +/* unit-flash-write-cc26x2.c + * + * Regression test for F-6869: hal_flash_write() and hal_flash_erase() in + * hal/cc26x2.c discarded the Fapi status returned by FlashProgram() and + * FlashSectorErase() and unconditionally returned 0, so a program or erase + * that the flash controller rejected was reported to wolfBoot as a success. + * Both now return -1 on any status other than FAPI_STATUS_SUCCESS. + * + * hal/cc26x2.c has no in-repo build target, so this test doubles as the + * only compile coverage the file gets: it includes it directly, with + * cc26x2_ti_stub/ standing in for the (not vendored) TI CC26x2 SDK headers. + * + * 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 +#include +#include + +#include "ti-lib.h" +#include "image.h" + +#define FLASH_BASE 0x00010000UL +#define ERASE_LOG_MAX 16 + +/* Injected Fapi status for the next FlashProgram() call. */ +static uint32_t program_status; +/* Injected Fapi status for the Nth FlashSectorErase() call (1-based); every + * other call returns FAPI_STATUS_SUCCESS. 0 disables the injection. */ +static int erase_fail_on; + +static int program_calls; +static uint32_t program_addr; +static uint32_t program_len; +static const uint8_t *program_src; + +static int erase_calls; +static uint32_t erase_addr[ERASE_LOG_MAX]; + +uint32_t FlashProgram(uint8_t *pui8DataBuffer, uint32_t ui32Address, + uint32_t ui32Count) +{ + program_calls++; + program_src = pui8DataBuffer; + program_addr = ui32Address; + program_len = ui32Count; + return program_status; +} + +uint32_t FlashSectorErase(uint32_t ui32SectorAddress) +{ + erase_calls++; + if (erase_calls <= ERASE_LOG_MAX) + erase_addr[erase_calls - 1] = ui32SectorAddress; + if (erase_fail_on != 0 && erase_calls == erase_fail_on) + return FAPI_STATUS_FSM_ERROR; + return FAPI_STATUS_SUCCESS; +} + +/* The FSM is always idle on the host: the driver spins on this until it + * reports ready. */ +uint32_t FlashCheckFsmForReady(void) +{ + return FAPI_STATUS_FSM_READY; +} + +/* Referenced by uart_read()/hal_init(), neither of which this test calls. */ +int32_t UARTCharGet(uint32_t ui32Base) { (void)ui32Base; return 0; } +int32_t UARTCharGetNonBlocking(uint32_t ui32Base) { (void)ui32Base; return -1; } +void clock_init(void) { } + +#include "../../hal/cc26x2.c" + +static void setup(void) +{ + program_status = FAPI_STATUS_SUCCESS; + erase_fail_on = 0; + program_calls = 0; + program_addr = 0; + program_len = 0; + program_src = NULL; + erase_calls = 0; + memset(erase_addr, 0, sizeof(erase_addr)); +} + +static void teardown(void) +{ +} + +/* A successful program still returns 0, and the arguments reach driverlib + * unchanged. */ +START_TEST(test_write_success) +{ + uint8_t data[8]; + int i; + + for (i = 0; i < 8; i++) + data[i] = (uint8_t)(i + 1); + + ck_assert_int_eq(hal_flash_write(FLASH_BASE, data, sizeof(data)), 0); + ck_assert_int_eq(program_calls, 1); + ck_assert_uint_eq(program_addr, FLASH_BASE); + ck_assert_uint_eq(program_len, sizeof(data)); + ck_assert_ptr_eq(program_src, data); +} +END_TEST + +/* A rejected program must be reported as a failure. Before the fix this + * returned 0 and wolfBoot went on to treat the unwritten page as valid. */ +START_TEST(test_write_fsm_error) +{ + uint8_t data[8]; + + memset(data, 0xA5, sizeof(data)); + program_status = FAPI_STATUS_FSM_ERROR; + + ck_assert_int_eq(hal_flash_write(FLASH_BASE, data, sizeof(data)), -1); + ck_assert_int_eq(program_calls, 1); +} +END_TEST + +/* Every other non-success status is a failure too, not just FSM_ERROR. */ +START_TEST(test_write_fsm_busy) +{ + uint8_t data[4]; + + memset(data, 0x5A, sizeof(data)); + program_status = FAPI_STATUS_FSM_BUSY; + + ck_assert_int_eq(hal_flash_write(FLASH_BASE, data, sizeof(data)), -1); +} +END_TEST + +/* A multi-sector erase walks the sectors in order and returns 0. */ +START_TEST(test_erase_success_multi_sector) +{ + int i; + + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, 3 * WOLFBOOT_SECTOR_SIZE), 0); + ck_assert_int_eq(erase_calls, 3); + for (i = 0; i < 3; i++) + ck_assert_uint_eq(erase_addr[i], + FLASH_BASE + (uint32_t)(WOLFBOOT_SECTOR_SIZE * i)); +} +END_TEST + +/* A length shorter than one sector still erases exactly one sector. */ +START_TEST(test_erase_success_partial_sector) +{ + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, WOLFBOOT_SECTOR_SIZE / 2), 0); + ck_assert_int_eq(erase_calls, 1); + ck_assert_uint_eq(erase_addr[0], FLASH_BASE); +} +END_TEST + +/* A rejected sector erase fails the whole call and stops immediately, rather + * than erasing on and returning 0 as it did before the fix. */ +START_TEST(test_erase_fsm_error_stops) +{ + erase_fail_on = 2; + + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, 4 * WOLFBOOT_SECTOR_SIZE), -1); + ck_assert_int_eq(erase_calls, 2); +} +END_TEST + +/* The very first sector failing must not be mistaken for "nothing to do". */ +START_TEST(test_erase_fsm_error_first_sector) +{ + erase_fail_on = 1; + + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, WOLFBOOT_SECTOR_SIZE), -1); + ck_assert_int_eq(erase_calls, 1); +} +END_TEST + +Suite *flash_write_suite(void) +{ + Suite *s = suite_create("flash-write-cc26x2"); + TCase *tc = tcase_create("flash-write-cc26x2"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_write_success); + tcase_add_test(tc, test_write_fsm_error); + tcase_add_test(tc, test_write_fsm_busy); + tcase_add_test(tc, test_erase_success_multi_sector); + tcase_add_test(tc, test_erase_success_partial_sector); + tcase_add_test(tc, test_erase_fsm_error_stops); + tcase_add_test(tc, test_erase_fsm_error_first_sector); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = flash_write_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +}