From 74530753b40150080f61cfd9277b1fa7d4e3fa6f Mon Sep 17 00:00:00 2001
From: Fabio Utzig <utzig@apache.org>
Date: Tue, 7 Feb 2023 20:09:37 -0300
Subject: [PATCH] bootutil: fix swap with move reset issue

Fix a swap corruption which occurs on the swap move algorithm when a
reset happens exactly at the point after the last move up, and its
status update. On restart the image headers should be read at the 2nd
sector of the primary slot, but due to lacking initialization it is
read on the first sector, and then fails. This error was masked on the
simulator because of the use of a global variable, which retained its
value on a "reset simulation".

Fixes #1588

Signed-off-by: Fabio Utzig <utzig@apache.org>
---
 boot/bootutil/src/swap_move.c | 78 +++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 31 deletions(-)

diff --git a/boot/bootutil/src/swap_move.c b/boot/bootutil/src/swap_move.c
index 6f3398270..a063a0cd5 100644
--- a/boot/bootutil/src/swap_move.c
+++ b/boot/bootutil/src/swap_move.c
@@ -47,7 +47,26 @@ int boot_status_fails = 0;
 #define BOOT_STATUS_ASSERT(x) ASSERT(x)
 #endif
 
-static uint32_t g_last_idx = UINT32_MAX;
+uint32_t
+find_last_idx(struct boot_loader_state *state, uint32_t swap_size)
+{
+    uint32_t sector_sz;
+    uint32_t sz;
+    uint32_t last_idx;
+
+    sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
+    sz = 0;
+    last_idx = 0;
+    while (1) {
+        sz += sector_sz;
+        last_idx++;
+        if (sz >= swap_size) {
+            break;
+        }
+    }
+
+    return last_idx;
+}
 
 int
 boot_read_image_header(struct boot_loader_state *state, int slot,
@@ -56,6 +75,8 @@ boot_read_image_header(struct boot_loader_state *state, int slot,
     const struct flash_area *fap;
     uint32_t off;
     uint32_t sz;
+    uint32_t last_idx;
+    uint32_t swap_size;
     int area_id;
     int rc;
 
@@ -64,25 +85,29 @@ boot_read_image_header(struct boot_loader_state *state, int slot,
 #endif
 
     off = 0;
-    if (bs) {
+    if (bs && !boot_status_is_reset(bs)) {
+        rc = boot_read_swap_size(BOOT_CURR_IMG(state), &swap_size);
+        if (rc) {
+            rc = BOOT_EFLASH;
+            goto done;
+        }
+
+        last_idx = find_last_idx(state, swap_size);
         sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
-        if (bs->op == BOOT_STATUS_OP_MOVE) {
-            if (slot == 0 && bs->idx > g_last_idx) {
-                /* second sector */
-                off = sz;
-            }
+
+        /*
+         * Find the correct offset or slot where the image header is expected to
+         * be found for the steps where it is moved or swapped.
+         */
+        if (bs->op == BOOT_STATUS_OP_MOVE && slot == 0 && bs->idx > last_idx) {
+            off = sz;
         } else if (bs->op == BOOT_STATUS_OP_SWAP) {
-            if (bs->idx > 1 && bs->idx <= g_last_idx) {
-                if (slot == 0) {
-                    slot = 1;
-                } else {
-                    slot = 0;
-                }
+            if (bs->idx > 1 && bs->idx <= last_idx) {
+                slot = (slot == 0) ? 1 : 0;
             } else if (bs->idx == 1) {
                 if (slot == 0) {
                     off = sz;
-                }
-                if (slot == 1 && bs->state == 2) {
+                } else if (slot == 1 && bs->state == 2) {
                     slot = 0;
                 }
             }
@@ -444,6 +469,7 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
     uint32_t idx;
     uint32_t trailer_sz;
     uint32_t first_trailer_idx;
+    uint32_t last_idx;
     uint8_t image_index;
     const struct flash_area *fap_pri;
     const struct flash_area *fap_sec;
@@ -451,18 +477,8 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
 
     BOOT_LOG_INF("Starting swap using move algorithm.");
 
-    sz = 0;
-    g_last_idx = 0;
-
+    last_idx = find_last_idx(state, copy_size);
     sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
-    while (1) {
-        sz += sector_sz;
-        /* Skip to next sector because all sectors will be moved up. */
-        g_last_idx++;
-        if (sz >= copy_size) {
-            break;
-        }
-    }
 
     /*
      * When starting a new swap upgrade, check that there is enough space.
@@ -480,10 +496,10 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
             first_trailer_idx--;
         }
 
-        if (g_last_idx >= first_trailer_idx) {
+        if (last_idx >= first_trailer_idx) {
             BOOT_LOG_WRN("Not enough free space to run swap upgrade");
             BOOT_LOG_WRN("required %d bytes but only %d are available",
-                         (g_last_idx + 1) * sector_sz ,
+                         (last_idx + 1) * sector_sz,
                          first_trailer_idx * sector_sz);
             bs->swap_type = BOOT_SWAP_TYPE_NONE;
             return;
@@ -501,9 +517,9 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
     fixup_revert(state, bs, fap_sec);
 
     if (bs->op == BOOT_STATUS_OP_MOVE) {
-        idx = g_last_idx;
+        idx = last_idx;
         while (idx > 0) {
-            if (idx <= (g_last_idx - bs->idx + 1)) {
+            if (idx <= (last_idx - bs->idx + 1)) {
                 boot_move_sector_up(idx, sector_sz, state, bs, fap_pri, fap_sec);
             }
             idx--;
@@ -514,7 +530,7 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
     bs->op = BOOT_STATUS_OP_SWAP;
 
     idx = 1;
-    while (idx <= g_last_idx) {
+    while (idx <= last_idx) {
         if (idx >= bs->idx) {
             boot_swap_sectors(idx, sector_sz, state, bs, fap_pri, fap_sec);
         }