Merge pull request #470 from danielinux/nvm-writeonce-flags-compare

Fixed flag comparison selecting most recent flags

Author: dgarske

.github/workflows/test-build.yml

@@ -17,7 +17,7 @@ on:
 jobs:
 
   build:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
 
     steps:
       - uses: actions/checkout@v3

hal/sim.c

@@ -116,9 +116,11 @@ int hal_flash_write(uintptr_t address, const uint8_t *data, int len)
     else {
         for (i = 0; i < len; i++) {
 #ifdef NVM_FLASH_WRITEONCE
-            if (((uint8_t*)address)[i] != FLASH_BYTE_ERASED) {
+            uint8_t *addr = (uint8_t *)address;
+            if (addr[i] != FLASH_BYTE_ERASED) {
                 /* no writing to non-erased page in NVM_FLASH_WRITEONCE */
-                printf("NVM_FLASH_WRITEONCE non-erased write detected!\n");
+                printf("NVM_FLASH_WRITEONCE non-erased write detected at address %p!\n", addr);
+                printf("Address[%d] = %02x\n", i, addr[i]);
                 return -1;
             }
 #endif

src/libwolfboot.c

@@ -27,7 +27,6 @@
  */
 #include <stdint.h>
 
-
 #include "hal.h"
 #include "wolfboot/wolfboot.h"
 #include "image.h"
@@ -191,56 +190,37 @@ static int RAMFUNCTION nvm_select_fresh_sector(int part)
     int sel;
     uintptr_t off;
     uint8_t *base;
-    uint8_t* addrErase;
+    uint8_t* addrErase = 0;
     uint32_t word_0;
     uint32_t word_1;
 
+#if defined(EXT_FLASH) && !defined(FLAGS_HOME)
+    if ((part == PART_UPDATE) && FLAGS_UPDATE_EXT()) {
+        return 0;
+    }
+#endif
+
     hal_cache_invalidate();
 
-    /* if FLAGS_HOME check both boot and update for changes */
-#ifdef FLAGS_HOME
-    base = (uint8_t *)PART_BOOT_ENDFLAGS;
-    addrErase = (uint8_t *)WOLFBOOT_PARTITION_BOOT_ADDRESS +
-        WOLFBOOT_PARTITION_SIZE - WOLFBOOT_SECTOR_SIZE;
-#else
     if (part == PART_BOOT) {
         base = (uint8_t *)PART_BOOT_ENDFLAGS;
         addrErase = (uint8_t *)WOLFBOOT_PARTITION_BOOT_ADDRESS +
             WOLFBOOT_PARTITION_SIZE - WOLFBOOT_SECTOR_SIZE;
     }
     else {
         base = (uint8_t *)PART_UPDATE_ENDFLAGS;
+#ifdef FLAGS_HOME
+        addrErase = (uint8_t *)WOLFBOOT_PARTITION_BOOT_ADDRESS +
+            WOLFBOOT_PARTITION_SIZE - WOLFBOOT_SECTOR_SIZE;
+#else
         addrErase = (uint8_t *)WOLFBOOT_PARTITION_UPDATE_ADDRESS +
             WOLFBOOT_PARTITION_SIZE - WOLFBOOT_SECTOR_SIZE;
-    }
-#endif
-
-#ifdef EXT_ENCRYPTED
-#ifndef FLAGS_HOME
-    if (part == PART_BOOT)
 #endif
-    {
-        word_0 = *((uint32_t *)(ENCRYPT_TMP_SECRET_OFFSET +
-            WOLFBOOT_PARTITION_BOOT_ADDRESS));
-        word_1 = *((uint32_t *)(ENCRYPT_TMP_SECRET_OFFSET +
-            WOLFBOOT_PARTITION_BOOT_ADDRESS - WOLFBOOT_SECTOR_SIZE));
-
-        if (word_0 == FLASH_WORD_ERASED && word_1 !=
-            FLASH_WORD_ERASED) {
-            sel = 1;
-            goto finish;
-        }
-        else if (word_0 != FLASH_WORD_ERASED && word_1 ==
-            FLASH_WORD_ERASED) {
-            sel = 0;
-            goto finish;
-        }
     }
-#endif
 
     /* check magic in case the sector is corrupt */
-    word_0 = *((uint32_t*)((uintptr_t)base - sizeof(uint32_t)));
-    word_1 = *((uint32_t*)((uintptr_t)base - WOLFBOOT_SECTOR_SIZE - sizeof(uint32_t)));
+    word_0 = *((uint32_t*)((uintptr_t)base -  sizeof(uint32_t)));
+    word_1 = *((uint32_t*)((uintptr_t)base - (WOLFBOOT_SECTOR_SIZE + sizeof(uint32_t))));
 
     if (word_0 == WOLFBOOT_MAGIC_TRAIL && word_1 != WOLFBOOT_MAGIC_TRAIL) {
         sel = 0;
@@ -249,32 +229,21 @@ static int RAMFUNCTION nvm_select_fresh_sector(int part)
     else if (word_0 != WOLFBOOT_MAGIC_TRAIL && word_1 == WOLFBOOT_MAGIC_TRAIL) {
         sel = 1;
         goto finish;
-    }
-
-/* try the update magic as well */
-#ifdef FLAGS_HOME
-    /* check magic in case the sector is corrupt */
-    word_0 = *((uint32_t*)(PART_UPDATE_ENDFLAGS - sizeof(uint32_t)));
-    word_1 = *((uint32_t*)(PART_UPDATE_ENDFLAGS - WOLFBOOT_SECTOR_SIZE -
-        sizeof(uint32_t)));
-
-    if (word_0 == WOLFBOOT_MAGIC_TRAIL && word_1 != WOLFBOOT_MAGIC_TRAIL) {
+    } else if (word_0 != WOLFBOOT_MAGIC_TRAIL && word_1 != WOLFBOOT_MAGIC_TRAIL) {
+        /* none of the partition has a valid trailer, default to '0' */
         sel = 0;
         goto finish;
     }
-    else if (word_0 != WOLFBOOT_MAGIC_TRAIL && word_1 == WOLFBOOT_MAGIC_TRAIL) {
-        sel = 1;
-        goto finish;
-    }
-#endif
 
     /* Default to last sector if no match is found */
     sel = 0;
 
-    /* Select the sector with more flags set */
-    for (off = 1; off < WOLFBOOT_SECTOR_SIZE; off++) {
-        uint8_t byte_0 = get_base_offset(base, off);
-        uint8_t byte_1 = get_base_offset(base, (WOLFBOOT_SECTOR_SIZE + off));
+    /* Select the sector with more flags set. Partition flag is at offset '4'.
+     * Sector flags begin from offset '5'. 
+     */
+    for (off = 4; off < WOLFBOOT_SECTOR_SIZE; off++) {
+        volatile uint8_t byte_0 = get_base_offset(base, off);
+        volatile uint8_t byte_1 = get_base_offset(base, (WOLFBOOT_SECTOR_SIZE + off));
 
         if (byte_0 == FLASH_BYTE_ERASED && byte_1 != FLASH_BYTE_ERASED) {
             sel = 1;
@@ -286,19 +255,6 @@ static int RAMFUNCTION nvm_select_fresh_sector(int part)
         }
         else if ((byte_0 == FLASH_BYTE_ERASED) &&
                 (byte_1 == FLASH_BYTE_ERASED)) {
-#ifdef FLAGS_HOME
-            /* if we're still checking boot flags, check update flags */
-            if (base - off > (uint8_t*)PART_UPDATE_ENDFLAGS) {
-                base = (uint8_t *)PART_UPDATE_ENDFLAGS;
-                off = 0;
-                continue;
-            }
-#endif
-            /* First time boot?  Assume no pending update */
-            if (off == 1) {
-                sel=0;
-                break;
-            }
             /* Examine previous position one byte ahead */
             byte_0 = get_base_offset(base, (off - 1));
             byte_1 = get_base_offset(base, ((WOLFBOOT_SECTOR_SIZE + off) - 1));
@@ -376,6 +332,7 @@ static int RAMFUNCTION partition_magic_write(uint8_t part, uintptr_t addr)
     uintptr_t base = (uintptr_t)addr - off;
     uintptr_t addr_read, addr_write;
     int ret;
+
     nvm_cached_sector = nvm_select_fresh_sector(part);
     addr_read = base - (nvm_cached_sector * NVM_CACHE_SIZE);
     addr_write = base - (!nvm_cached_sector * NVM_CACHE_SIZE);
@@ -766,8 +723,7 @@ void RAMFUNCTION wolfBoot_erase_partition(uint8_t part)
 void RAMFUNCTION wolfBoot_update_trigger(void)
 {
     uint8_t st = IMG_STATE_UPDATING;
-    uintptr_t lastSector = PART_UPDATE_ENDFLAGS -
-        (PART_UPDATE_ENDFLAGS % WOLFBOOT_SECTOR_SIZE);
+    uintptr_t lastSector = ((PART_UPDATE_ENDFLAGS - 1) / WOLFBOOT_SECTOR_SIZE) * WOLFBOOT_SECTOR_SIZE;
 #ifdef NVM_FLASH_WRITEONCE
     uint8_t selSec = 0;
 #endif
@@ -795,11 +751,10 @@ void RAMFUNCTION wolfBoot_update_trigger(void)
         hal_flash_erase(lastSector, SECTOR_FLAGS_SIZE);
 #else
         selSec = nvm_select_fresh_sector(PART_UPDATE);
-        XMEMCPY(NVM_CACHE,
-            (uint8_t*)(lastSector - WOLFBOOT_SECTOR_SIZE * selSec),
-            WOLFBOOT_SECTOR_SIZE);
-        XMEMSET(NVM_CACHE, FLASH_BYTE_ERASED, SECTOR_FLAGS_SIZE);
+        lastSector -= selSec * WOLFBOOT_SECTOR_SIZE;
+        XMEMCPY(NVM_CACHE, (uint8_t*)lastSector, WOLFBOOT_SECTOR_SIZE);
         /* write to the non selected sector */
+        hal_flash_erase(lastSector - WOLFBOOT_SECTOR_SIZE * !selSec, WOLFBOOT_SECTOR_SIZE);
         hal_flash_write(lastSector - WOLFBOOT_SECTOR_SIZE * !selSec, NVM_CACHE,
             WOLFBOOT_SECTOR_SIZE);
         /* erase the previously selected sector */
@@ -1391,6 +1346,7 @@ static int RAMFUNCTION hal_set_key(const uint8_t *k, const uint8_t *nonce)
     uintptr_t addr, addr_align, addr_off;
     int ret = 0;
     int sel_sec = 0;
+    uint32_t trailer_relative_off = 4;
 #ifdef MMU
     XMEMCPY(ENCRYPT_KEY, k, ENCRYPT_KEY_SIZE);
     XMEMCPY(ENCRYPT_KEY + ENCRYPT_KEY_SIZE, nonce, ENCRYPT_NONCE_SIZE);
@@ -1419,14 +1375,33 @@ static int RAMFUNCTION hal_set_key(const uint8_t *k, const uint8_t *nonce)
     if (ret != 0)
         return ret;
 #endif
+
+    /* Populate key + nonce in the cache */
     XMEMCPY(ENCRYPT_CACHE + addr_off, k, ENCRYPT_KEY_SIZE);
     XMEMCPY(ENCRYPT_CACHE + addr_off + ENCRYPT_KEY_SIZE, nonce,
         ENCRYPT_NONCE_SIZE);
+
+    /* Add a valid trailer */
+    XMEMCPY(ENCRYPT_CACHE + addr_off - trailer_relative_off,
+            &wolfboot_magic_trail, 4);
+#ifdef FLAGS_HOME
+    /* If flags are stored in BOOT partition, take into account the offset
+     * of the flags used for the update partition too, to avoid erasing the
+     * sector.
+     */
+    trailer_relative_off += (PART_BOOT_ENDFLAGS - PART_UPDATE_ENDFLAGS);
+    XMEMCPY(ENCRYPT_CACHE + addr_off - trailer_relative_off,
+            &wolfboot_magic_trail, 4);
+#endif
+
+    /* Writing cache back to sector "!sel_sec" */
     ret = hal_flash_write(addr_align, ENCRYPT_CACHE, WOLFBOOT_SECTOR_SIZE);
 #ifdef NVM_FLASH_WRITEONCE
-    /* now erase the old populated sector */
     if (ret != 0)
         return ret;
+    /* Erasing original sector "sel_sec",
+     * same one returned from by nvm_select.
+     */
     addr_align = addr & (~(WOLFBOOT_SECTOR_SIZE - 1));
     addr_align -= (sel_sec * WOLFBOOT_SECTOR_SIZE);
     ret = hal_flash_erase(addr_align, WOLFBOOT_SECTOR_SIZE);
@@ -1508,7 +1483,7 @@ int RAMFUNCTION wolfBoot_erase_encrypt_key(void)
     sel_sec = nvm_select_fresh_sector(PART_BOOT);
     mem -= (sel_sec * WOLFBOOT_SECTOR_SIZE);
 #endif
-    XMEMSET(ff, 0xFF, ENCRYPT_KEY_SIZE + ENCRYPT_NONCE_SIZE);
+    XMEMSET(ff, FLASH_BYTE_ERASED, ENCRYPT_KEY_SIZE + ENCRYPT_NONCE_SIZE);
     if (XMEMCMP(mem, ff, ENCRYPT_KEY_SIZE + ENCRYPT_NONCE_SIZE) != 0)
         hal_set_key(ff, ff + ENCRYPT_KEY_SIZE);
 #endif
@@ -1717,10 +1692,6 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data,
     if (sz < ENCRYPT_BLOCK_SIZE) {
         sz = ENCRYPT_BLOCK_SIZE;
     }
-    if (!encrypt_initialized) {
-        if (crypto_init() < 0)
-            return -1;
-    }
     part = part_address(address);
     switch (part) {
         case PART_UPDATE:
@@ -1731,6 +1702,10 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data,
                     ENCRYPT_BLOCK_SIZE) {
                 return ext_flash_write(address, data, len);
             }
+            if (!encrypt_initialized) {
+                if (crypto_init() < 0)
+                    return -1;
+            }
             crypto_set_iv(encrypt_iv_nonce, iv_counter);
             break;
         case PART_SWAP:
@@ -1799,10 +1774,6 @@ int RAMFUNCTION ext_flash_decrypt_read(uintptr_t address, uint8_t *data, int len
     if (row_offset != 0) {
         row_address = address & ~(ENCRYPT_BLOCK_SIZE - 1);
     }
-    if (!encrypt_initialized) {
-        if (crypto_init() < 0)
-            return -1;
-    }
     part = part_address(row_address);
     switch (part) {
         case PART_UPDATE:
@@ -1813,6 +1784,11 @@ int RAMFUNCTION ext_flash_decrypt_read(uintptr_t address, uint8_t *data, int len
                     ENCRYPT_BLOCK_SIZE) {
                 return ext_flash_read(address, data, len);
             }
+            if (!encrypt_initialized) {
+                if (crypto_init() < 0) {
+                    return -1;
+                }
+            }
             crypto_set_iv(encrypt_iv_nonce, iv_counter);
             break;
         case PART_SWAP:
@@ -1894,18 +1870,15 @@ int wolfBoot_ram_decrypt(uint8_t *src, uint8_t *dst)
     uint32_t dst_offset = 0, iv_counter = 0;
     uint32_t magic, len;
 
-    wolfBoot_printf("Decrypting %p to %p\n", src, dst);
 
     if (!encrypt_initialized) {
         if (crypto_init() < 0) {
-            wolfBoot_printf("Error initializing crypto!\n");
             return -1;
         }
     }
 
     /* Attempt to decrypt firmware header */
     if (decrypt_header(src) != 0) {
-        wolfBoot_printf("Error decrypting header at %p!\n", src);
         return -1;
     }
     len = *((uint32_t*)(dec_hdr + sizeof(uint32_t)));

src/update_flash.c

@@ -501,6 +501,7 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed)
     uint32_t up_v;
 #endif
 
+
     /* No Safety check on open: we might be in the middle of a broken update */
     wolfBoot_open_image(&update, PART_UPDATE);
     wolfBoot_open_image(&boot, PART_BOOT);
@@ -819,7 +820,8 @@ void RAMFUNCTION wolfBoot_start(void)
     bootRet = wolfBoot_get_partition_state(PART_BOOT, &bootState);
     updateRet = wolfBoot_get_partition_state(PART_UPDATE, &updateState);
 
-#ifndef DISABLE_BACKUP
+
+#if !defined(DISABLE_BACKUP)
     /* resume the final erase in case the power failed before it finished */
     resumedFinalErase = wolfBoot_swap_and_final_erase(1);
     if (resumedFinalErase != 0)

test-app/ARM-stm32f7.ld

@@ -17,6 +17,11 @@ SECTIONS
         . = ALIGN(4);
         _end_text = .;
     } > FLASH
+    .edidx :
+    {
+        . = ALIGN(4);
+        *(.ARM.exidx*)
+    } > FLASH
 
     _stored_data = .;
 

test-app/app_sim.c

@@ -61,6 +61,12 @@ int do_cmd(const char *cmd)
         printf("%d\n", wolfBoot_current_firmware_version());
         return 0;
     }
+    if (strcmp(cmd, "get_state") == 0) {
+        uint8_t st = 0;
+        wolfBoot_get_partition_state(PART_UPDATE, &st);
+        printf("%02x\n", st);
+        return 0;
+    }
     if (strcmp(cmd, "success") == 0) {
         wolfBoot_success();
         return 0;

tools/lms/lms_common.h

@@ -48,8 +48,7 @@ static int lms_write_key(const byte * priv, word32 privSz, void * context)
         /* Create the file if it didn't exist. */
         file = fopen(filename, "w+");
         if (!file) {
-            fprintf(stderr, "error: fopen(%s, \"w+\") failed: %d\n", filename,
-                    ferror(file));
+            fprintf(stderr, "error: fopen(%s, \"w+\") failed.\n", filename);
             return WC_LMS_RC_WRITE_FAIL;
         }
     }
@@ -72,8 +71,7 @@ static int lms_write_key(const byte * priv, word32 privSz, void * context)
      * storage correctly. */
     file = fopen(filename, "r+");
     if (!file) {
-        fprintf(stderr, "error: fopen(%s, \"r+\") failed: %d\n", filename,
-                ferror(file));
+        fprintf(stderr, "error: fopen(%s, \"r+\") failed.\n", filename);
         return WC_LMS_RC_WRITE_FAIL;
     }
 

tools/scripts/sim-sunnyday-update.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
-
 V=`./wolfboot.elf update_trigger get_version 2>/dev/null`
 if [ "x$V" != "x1" ]; then
     echo "Failed first boot with update_trigger"