From c4e853ea4b13cc3bcbf701f4f648afeb0fbe095f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 16:17:51 +0200 Subject: [PATCH] F-6131: enforce full-transfer bounds check in spi_flash_read/spi_flash_write (QSPI) spi_flash_read only rejected address > FLASH_DEVICE_SIZE (off-by-one, admits address == FLASH_DEVICE_SIZE) and never validated address+len against the device size, so an in-bounds start with an out-of-range extent was not rejected. spi_flash_write had no bounds check at all. Add an inclusive/full-extent check to both. --- src/qspi_flash.c | 16 +++++++++-- tools/unit-tests/unit-qspi-flash.c | 46 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/qspi_flash.c b/src/qspi_flash.c index 6a446af2..bc6095b7 100644 --- a/src/qspi_flash.c +++ b/src/qspi_flash.c @@ -389,10 +389,11 @@ int spi_flash_read(uint32_t address, void *data, int len) uint32_t altMode = QSPI_DATA_MODE_NONE; #endif - if (address > FLASH_DEVICE_SIZE) { + if ((len < 0) || (address >= FLASH_DEVICE_SIZE) || + ((uint64_t)address + (uint32_t)len > FLASH_DEVICE_SIZE)) { #ifdef DEBUG_QSPI - wolfBoot_printf("QSPI Flash Read: Invalid address (0x%x > 0x%x max)\n", - address, FLASH_DEVICE_SIZE); + wolfBoot_printf("QSPI Flash Read: Invalid address (0x%x, len %d, max 0x%x)\n", + address, len, FLASH_DEVICE_SIZE); #endif return -1; } @@ -427,6 +428,15 @@ int spi_flash_write(uint32_t address, const void *data, int len) len, data, address); #endif + if ((len < 0) || (address >= FLASH_DEVICE_SIZE) || + ((uint64_t)address + (uint32_t)len > FLASH_DEVICE_SIZE)) { +#ifdef DEBUG_QSPI + wolfBoot_printf("QSPI Flash Write: Invalid address (0x%x, len %d, max 0x%x)\n", + address, len, FLASH_DEVICE_SIZE); +#endif + return -1; + } + /* write by page */ pages = ((len + (FLASH_PAGE_SIZE-1)) / FLASH_PAGE_SIZE); for (page = 0; page < pages; page++) { diff --git a/tools/unit-tests/unit-qspi-flash.c b/tools/unit-tests/unit-qspi-flash.c index 17e1bf16..344c7272 100644 --- a/tools/unit-tests/unit-qspi-flash.c +++ b/tools/unit-tests/unit-qspi-flash.c @@ -17,6 +17,7 @@ static uint32_t program_addrs[8]; static int write_enable_call_count; static int write_enable_status_seq[8]; static int current_write_enable_call; +static int read_call_count; void spi_init(int polarity, int phase) { @@ -72,6 +73,11 @@ int qspi_transfer(uint8_t fmode, const uint8_t cmd, return 0; } + if (cmd == FLASH_READ_CMD) { + read_call_count++; + return 0; + } + return 0; } @@ -84,6 +90,7 @@ static void setup(void) write_enable_call_count = 0; current_write_enable_call = 0; memset(write_enable_status_seq, 0, sizeof(write_enable_status_seq)); + read_call_count = 0; } START_TEST(test_qspi_write_splits_last_page_to_remaining_bytes) @@ -124,6 +131,42 @@ START_TEST(test_qspi_write_stops_after_midloop_write_enable_failure) } END_TEST +START_TEST(test_qspi_read_rejects_address_at_device_size) +{ + uint8_t buf[16]; + int ret; + + ret = spi_flash_read(FLASH_DEVICE_SIZE, buf, sizeof(buf)); + + ck_assert_int_eq(ret, -1); + ck_assert_int_eq(read_call_count, 0); +} +END_TEST + +START_TEST(test_qspi_read_rejects_transfer_extending_past_device_size) +{ + uint8_t buf[16]; + int ret; + + ret = spi_flash_read(FLASH_DEVICE_SIZE - 4, buf, sizeof(buf)); + + ck_assert_int_eq(ret, -1); + ck_assert_int_eq(read_call_count, 0); +} +END_TEST + +START_TEST(test_qspi_write_rejects_transfer_extending_past_device_size) +{ + uint8_t buf[16]; + int ret; + + ret = spi_flash_write(FLASH_DEVICE_SIZE - 4, buf, sizeof(buf)); + + ck_assert_int_ne(ret, 0); + ck_assert_int_eq(program_call_count, 0); +} +END_TEST + static Suite *qspi_flash_suite(void) { Suite *s; @@ -134,6 +177,9 @@ static Suite *qspi_flash_suite(void) tcase_add_checked_fixture(tc, setup, NULL); tcase_add_test(tc, test_qspi_write_splits_last_page_to_remaining_bytes); tcase_add_test(tc, test_qspi_write_stops_after_midloop_write_enable_failure); + tcase_add_test(tc, test_qspi_read_rejects_address_at_device_size); + tcase_add_test(tc, test_qspi_read_rejects_transfer_extending_past_device_size); + tcase_add_test(tc, test_qspi_write_rejects_transfer_extending_past_device_size); suite_add_tcase(s, tc); return s; }