Address 3 Fenrir findings on PR 905

hal/nrf5340.c

@@ -827,7 +827,13 @@ void hal_init(void)
 }
 
 #ifdef __WOLFBOOT
-/* enable write protection for the region of flash specified */
+/* Enable write protection for the region of flash specified.
+ *
+ * Contract: protects [start, start+len). A zero len protects nothing and
+ * succeeds; a negative len is rejected. Protection is granted in whole
+ * SPU_FLASH_BLOCK_SIZE blocks, so a partial block at either end is locked
+ * whole - the locked range may be wider than requested, never narrower.
+ */
 int RAMFUNCTION hal_flash_protect(haladdr_t start, int len)
 {
     /* only application core supports SPU */
@@ -840,6 +846,11 @@ int RAMFUNCTION hal_flash_protect(haladdr_t start, int len)
         return -1;
     if (len < 0)
         return -1;
+    /* An empty range protects nothing. Return before the region math below:
+     * `tail` carries the start offset, so an unaligned start would round up
+     * to one block and lock 16 KiB the caller never asked to protect. */
+    if (len == 0)
+        return 0;
     /* truncate if exceeds flash size */
     if (start + (uint32_t)len > FLASH_SIZE)
         len = FLASH_SIZE - start;

tools/unit-tests/unit-nrf5340-flash-protect.c

@@ -31,6 +31,7 @@
  */
 
 #include <check.h>
+#include <limits.h>
 #include <stdint.h>
 #include <string.h>
 #include <unistd.h>
@@ -124,6 +125,37 @@ START_TEST(test_unaligned_start_covers_range)
 }
 END_TEST
 
+/* A zero-length range protects nothing, so no region may be locked - not
+ * even when start sits mid-block. The round-up introduced for F-13623 made
+ * `tail` carry the start offset, so an unaligned start with len 0 rounded up
+ * to one block and locked 16 KiB that the caller never asked for. */
+START_TEST(test_zero_len_locks_nothing)
+{
+    sim_reset();
+    ck_assert_int_eq(hal_flash_protect(0, 0), 0);
+    ck_assert_int_eq(locked_count(), 0);
+
+    sim_reset();
+    ck_assert_int_eq(hal_flash_protect(0x2000, 0), 0);
+    ck_assert_int_eq(locked_count(), 0);
+}
+END_TEST
+
+/* A negative len is rejected outright. Without the guard the cast to
+ * uint32_t turns -1 into ~4 GiB, which truncates to FLASH_SIZE and locks
+ * every region while still returning success. */
+START_TEST(test_negative_len_rejected)
+{
+    sim_reset();
+    ck_assert_int_eq(hal_flash_protect(0, -1), -1);
+    ck_assert_int_eq(locked_count(), 0);
+
+    sim_reset();
+    ck_assert_int_eq(hal_flash_protect(0x2000, INT_MIN), -1);
+    ck_assert_int_eq(locked_count(), 0);
+}
+END_TEST
+
 /* start past the end of flash is rejected and nothing is locked. */
 START_TEST(test_start_past_flash_rejected)
 {
@@ -154,6 +186,8 @@ Suite *wolfboot_suite(void)
     tcase_add_test(tc, test_sub_block_len_locks_containing_block);
     tcase_add_test(tc, test_partial_tail_block_is_locked);
     tcase_add_test(tc, test_unaligned_start_covers_range);
+    tcase_add_test(tc, test_zero_len_locks_nothing);
+    tcase_add_test(tc, test_negative_len_rejected);
     tcase_add_test(tc, test_start_past_flash_rejected);
     tcase_add_test(tc, test_range_past_flash_truncated);
     suite_add_tcase(s, tc);

tools/unit-tests/unit-nsc-update.c

@@ -43,11 +43,17 @@
 
 const char *argv0;
 
+/* Per-run backing file: a fixed path is rewritten (O_TRUNC|MAP_SHARED) by
+ * every concurrent run, so two suites in parallel would share one mapping.
+ * Set once in main() before Check forks, so the child that creates the file
+ * and the parent that unlinks it agree on the name. */
+static char update_flash_file[PATH_MAX];
+
 static void prepare_update_flash(void)
 {
     int ret;
 
-    ret = mmap_file("/tmp/wolfboot-nsc-update.bin",
+    ret = mmap_file(update_flash_file,
                     (void *)WOLFBOOT_PARTITION_UPDATE_ADDRESS,
                     WOLFBOOT_PARTITION_SIZE,
                     NULL);
@@ -126,6 +132,8 @@ int main(int argc, char *argv[])
     SRunner *sr;
 
     argv0 = strdup(argv[0]);
+    snprintf(update_flash_file, sizeof(update_flash_file),
+             "/tmp/wolfboot-unit-nsc-update-%d.bin", (int)getpid());
     s = wolfboot_suite();
     sr = srunner_create(s);
 #if (NO_FORK == 1)
@@ -134,5 +142,6 @@ int main(int argc, char *argv[])
     srunner_run_all(sr, CK_NORMAL);
     fails = srunner_ntests_failed(sr);
     srunner_free(sr);
+    unlink(update_flash_file);
     return fails;
 }