diff --git a/hal/nrf52.c b/hal/nrf52.c index 1412ac7a..9fad8220 100644 --- a/hal/nrf52.c +++ b/hal/nrf52.c @@ -73,11 +73,15 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)address; + /* Index by "i" directly: the condition above only guarantees + * that "address + i" and "data + i" are word aligned, so + * dst[i >> 2] off the unaligned base would address the wrong + * word (and fault on a strict-alignment core). */ + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i); NVMC_CONFIG = NVMC_CONFIG_WEN; flash_wait_complete(); - dst[i >> 2] = src[i >> 2]; + *dst = *src; flash_wait_complete(); i+=4; } else { diff --git a/hal/nrf5340.c b/hal/nrf5340.c index 79f2635a..f1762086 100644 --- a/hal/nrf5340.c +++ b/hal/nrf5340.c @@ -317,14 +317,18 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)address; + /* Index by "i" directly: the condition above only guarantees + * that "address + i" and "data + i" are word aligned, so + * dst[i >> 2] off the unaligned base would address the wrong + * word (and fault on a strict-alignment core). */ + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i); #if TZ_SECURE() || defined(TARGET_nrf5340_net) NVMC_CONFIG = NVMC_CONFIG_WEN; #endif NVMC_CONFIGNS = NVMC_CONFIG_WEN; while (NVMC_READY == 0); - dst[i >> 2] = src[i >> 2]; + *dst = *src; while (NVMC_READY == 0); i+=4; } else { diff --git a/hal/stm32l0.c b/hal/stm32l0.c index 59ffea70..5e508343 100644 --- a/hal/stm32l0.c +++ b/hal/stm32l0.c @@ -111,10 +111,14 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE); + /* Index by "i" directly: the condition above only guarantees + * that "address + i" and "data + i" are word aligned, so + * dst[i >> 2] off the unaligned base would address the wrong + * word, and the Cortex-M0+ faults on the unaligned access. */ + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i + FLASHMEM_ADDRESS_SPACE); flash_wait_complete(); - dst[i >> 2] = src[i >> 2]; + *dst = *src; flash_wait_complete(); i+=4; } else { diff --git a/tools/unit-tests/unit-flash-write-nrf52.c b/tools/unit-tests/unit-flash-write-nrf52.c index 18ac09f2..3872d3d0 100644 --- a/tools/unit-tests/unit-flash-write-nrf52.c +++ b/tools/unit-tests/unit-flash-write-nrf52.c @@ -140,6 +140,34 @@ START_TEST(test_unaligned_write_mismatched_alignment) } END_TEST +/* Destination and source share the same non-zero misalignment, so once the + * byte-wise path has advanced i to the next word boundary both fast-path + * conditions hold and the 32-bit branch is entered with i != 0. Before the + * fix that branch indexed dst[i >> 2]/src[i >> 2] off the unaligned bases, + * writing data[0..3] to "address..address+3" instead of data[3..6] to + * "address+3..address+6" -- through a misaligned 32-bit flash access. */ +START_TEST(test_unaligned_write_matching_alignment_fast_path) +{ + uint8_t rawbuf[64]; + uint8_t *data = rawbuf; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + while (((uintptr_t)data % 4) != 1) + data++; + for (i = 0; i < 12; i++) + data[i] = (uint8_t)(0xD0 + i); + + ck_assert_int_eq(hal_flash_write(base + 1, data, 12), 0); + + ck_assert_uint_eq(mock_flash[0], 0xFF); + for (i = 0; i < 12; i++) + ck_assert_uint_eq(mock_flash[1 + i], data[i]); + for (i = 13; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + /* A write that fits entirely inside a single flash word must still work: * buggy and fixed forms agree here (i is always 0 in the byte-wise path), * guarding against a fix that breaks the common case. */ @@ -170,6 +198,7 @@ Suite *flash_write_suite(void) tcase_add_checked_fixture(tc, setup, teardown); tcase_add_test(tc, test_aligned_write_unaligned_tail); tcase_add_test(tc, test_unaligned_write_mismatched_alignment); + tcase_add_test(tc, test_unaligned_write_matching_alignment_fast_path); tcase_add_test(tc, test_unaligned_write_single_word); suite_add_tcase(s, tc);