Fix double swap on interrupted revert
This fixes #480.
When mcuboot rewrites image trailers during a swap, some information is
lost. If a reset occurs before the swap completes, mcuboot may not be
able to determine what which swap type to resume upon startup.
Specifically, if a "revert" swap gets interupted, mcuboot will perform
an extraneous swap on the subsequent boot. See
https://github.com/JuulLabs-OSS/mcuboot/issues/480 for details.
This commit adds an additional field to the image trailer: `swap-type`.
The new trailer structure is illustrated below:
```
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~ ~
~ Swap status (BOOT_MAX_IMG_SECTORS * min-write-size * 3) ~
~ ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~ Encryption key 0 (16 octets) [*] ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~ Encryption key 1 (16 octets) [*] ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Swap size | 0xff padding (4 octets) |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Swap type | 0xff padding (7 octets) ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Copy done | 0xff padding (7 octets) ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Image OK | 0xff padding (7 octets) ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~ MAGIC (16 octets) ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
```
The `swap-type` field contains one of the `BOOT_SWAP_TYPE_[...]` constants.
Every time a trailer is written, this field is written along with it.
When resuming an interrupted swap, mcuboot uses this field alone to
determine the type of swap being resumed. For new swap operations
(non-resume case), this field is not read at all; instead, mcuboot
consults the `boot_swap_tables` array to determine the swap operation to
perform (as it did prior to this commit).
Some additional changes were necessary to make all the simulated unit
tests pass:
* Before initiating a new swap operation, always write the image trailer
to the scratch area. This step allows mcuboot to persist the
`swap-type` field somewhere before erasing the trailer in the primary
slot. If a reset occurs immediately after the erase, mcuboot recovers
by using the trailer in the scratch area.
* Related to the above: if the scratch area is being used to hold status
bytes (because there are no spare sectors in the primary slot), erase
the scratch area immediately after the trailer gets written to the
primary slot. This eliminates ambiguity regarding the location of the
current trailer in case a reset occurs shortly afterwards.
Signed-off-by: Christopher Collins <ccollins@apache.org>
diff --git a/boot/bootutil/include/bootutil/bootutil.h b/boot/bootutil/include/bootutil/bootutil.h
index 3e9a835..e9a8f12 100644
--- a/boot/bootutil/include/bootutil/bootutil.h
+++ b/boot/bootutil/include/bootutil/bootutil.h
@@ -79,10 +79,12 @@
* when attempting to read/write a trailer.
*/
struct image_trailer {
- uint8_t copy_done;
+ uint8_t swap_type;
uint8_t pad1[MAX_FLASH_ALIGN - 1];
- uint8_t image_ok;
+ uint8_t copy_done;
uint8_t pad2[MAX_FLASH_ALIGN - 1];
+ uint8_t image_ok;
+ uint8_t pad3[MAX_FLASH_ALIGN - 1];
uint8_t magic[16];
};
diff --git a/boot/bootutil/src/bootutil_misc.c b/boot/bootutil/src/bootutil_misc.c
index 0debf18..8cab4c4 100644
--- a/boot/bootutil/src/bootutil_misc.c
+++ b/boot/bootutil/src/bootutil_misc.c
@@ -119,6 +119,32 @@
return BOOT_FLAG_SET;
}
+/**
+ * Determines if a status source table is satisfied by the specified magic
+ * code.
+ *
+ * @param tbl_val A magic field from a status source table.
+ * @param val The magic value in a trailer, encoded as a
+ * BOOT_MAGIC_[...].
+ *
+ * @return 1 if the two values are compatible;
+ * 0 otherwise.
+ */
+int
+boot_magic_compatible_check(uint8_t tbl_val, uint8_t val)
+{
+ switch (tbl_val) {
+ case BOOT_MAGIC_ANY:
+ return 1;
+
+ case BOOT_MAGIC_NOTGOOD:
+ return val != BOOT_MAGIC_GOOD;
+
+ default:
+ return tbl_val == val;
+ }
+}
+
uint32_t
boot_trailer_sz(uint8_t min_write_sz)
{
@@ -136,7 +162,6 @@
static uint32_t
boot_magic_off(const struct flash_area *fap)
{
- assert(offsetof(struct image_trailer, magic) == 16);
return fap->fa_size - BOOT_MAGIC_SZ;
}
@@ -168,17 +193,21 @@
return fap->fa_size - off_from_end;
}
+uint32_t
+boot_swap_type_off(const struct flash_area *fap)
+{
+ return fap->fa_size - BOOT_MAGIC_SZ - BOOT_MAX_ALIGN * 3;
+}
+
static uint32_t
boot_copy_done_off(const struct flash_area *fap)
{
- assert(offsetof(struct image_trailer, copy_done) == 0);
return fap->fa_size - BOOT_MAGIC_SZ - BOOT_MAX_ALIGN * 2;
}
static uint32_t
boot_image_ok_off(const struct flash_area *fap)
{
- assert(offsetof(struct image_trailer, image_ok) == 8);
return fap->fa_size - BOOT_MAGIC_SZ - BOOT_MAX_ALIGN;
}
@@ -216,6 +245,16 @@
state->magic = boot_magic_decode(magic);
}
+ off = boot_swap_type_off(fap);
+ rc = flash_area_read_is_empty(fap, off, &state->swap_type,
+ sizeof state->swap_type);
+ if (rc < 0) {
+ return BOOT_EFLASH;
+ }
+ if (rc == 1 || state->swap_type > BOOT_SWAP_TYPE_REVERT) {
+ state->swap_type = BOOT_SWAP_TYPE_NONE;
+ }
+
off = boot_copy_done_off(fap);
rc = flash_area_read_is_empty(fap, off, &state->copy_done,
sizeof state->copy_done);
@@ -405,34 +444,19 @@
}
static int
-boot_write_flag(int flag, const struct flash_area *fap)
+boot_write_trailer_byte(const struct flash_area *fap, uint32_t off,
+ uint8_t val)
{
- uint32_t off;
- int rc;
uint8_t buf[BOOT_MAX_ALIGN];
uint8_t align;
uint8_t erased_val;
-
- switch (flag) {
- case BOOT_FLAG_COPY_DONE:
- off = boot_copy_done_off(fap);
- BOOT_LOG_DBG("writing copy_done; fa_id=%d off=0x%x (0x%x)",
- fap->fa_id, off, fap->fa_off + off);
- break;
- case BOOT_FLAG_IMAGE_OK:
- off = boot_image_ok_off(fap);
- BOOT_LOG_DBG("writing image_ok; fa_id=%d off=0x%x (0x%x)",
- fap->fa_id, off, fap->fa_off + off);
- break;
- default:
- return BOOT_EBADARGS;
- }
+ int rc;
align = flash_area_align(fap);
assert(align <= BOOT_MAX_ALIGN);
erased_val = flash_area_erased_val(fap);
memset(buf, erased_val, BOOT_MAX_ALIGN);
- buf[0] = BOOT_FLAG_SET;
+ buf[0] = val;
rc = flash_area_write(fap, off, buf, align);
if (rc != 0) {
@@ -445,13 +469,39 @@
int
boot_write_copy_done(const struct flash_area *fap)
{
- return boot_write_flag(BOOT_FLAG_COPY_DONE, fap);
+ uint32_t off;
+
+ off = boot_copy_done_off(fap);
+ BOOT_LOG_DBG("writing copy_done; fa_id=%d off=0x%x (0x%x)",
+ fap->fa_id, off, fap->fa_off + off);
+ return boot_write_trailer_byte(fap, off, BOOT_FLAG_SET);
}
int
boot_write_image_ok(const struct flash_area *fap)
{
- return boot_write_flag(BOOT_FLAG_IMAGE_OK, fap);
+ uint32_t off;
+
+ off = boot_image_ok_off(fap);
+ BOOT_LOG_DBG("writing image_ok; fa_id=%d off=0x%x (0x%x)",
+ fap->fa_id, off, fap->fa_off + off);
+ return boot_write_trailer_byte(fap, off, BOOT_FLAG_SET);
+}
+
+/**
+ * Writes the specified value to the `swap-type` field of an image trailer.
+ * This value is persisted so that the boot loader knows what swap operation to
+ * resume in case of an unexpected reset.
+ */
+int
+boot_write_swap_type(const struct flash_area *fap, uint8_t swap_type)
+{
+ uint32_t off;
+
+ off = boot_swap_type_off(fap);
+ BOOT_LOG_DBG("writing swap_type; fa_id=%d off=0x%x (0x%x), swap_type=0x%x",
+ fap->fa_id, off, fap->fa_off + off, swap_type);
+ return boot_write_trailer_byte(fap, off, swap_type);
}
int
@@ -524,10 +574,10 @@
for (i = 0; i < BOOT_SWAP_TABLES_COUNT; i++) {
table = boot_swap_tables + i;
- if ((table->magic_primary_slot == BOOT_MAGIC_ANY ||
- table->magic_primary_slot == primary_slot.magic) &&
- (table->magic_secondary_slot == BOOT_MAGIC_ANY ||
- table->magic_secondary_slot == secondary_slot.magic) &&
+ if (boot_magic_compatible_check(table->magic_primary_slot,
+ primary_slot.magic) &&
+ boot_magic_compatible_check(table->magic_secondary_slot,
+ secondary_slot.magic) &&
(table->image_ok_primary_slot == BOOT_FLAG_ANY ||
table->image_ok_primary_slot == primary_slot.image_ok) &&
(table->image_ok_secondary_slot == BOOT_FLAG_ANY ||
@@ -566,6 +616,7 @@
{
const struct flash_area *fap;
struct boot_swap_state state_secondary_slot;
+ uint8_t swap_type;
int rc;
rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_SECONDARY,
@@ -591,6 +642,15 @@
rc = boot_write_image_ok(fap);
}
+ if (rc == 0) {
+ if (permanent) {
+ swap_type = BOOT_SWAP_TYPE_PERM;
+ } else {
+ swap_type = BOOT_SWAP_TYPE_TEST;
+ }
+ rc = boot_write_swap_type(fap, swap_type);
+ }
+
flash_area_close(fap);
return rc;
diff --git a/boot/bootutil/src/bootutil_priv.h b/boot/bootutil/src/bootutil_priv.h
index a5ca42f..d2871ee 100644
--- a/boot/bootutil/src/bootutil_priv.h
+++ b/boot/bootutil/src/bootutil_priv.h
@@ -61,6 +61,7 @@
uint32_t idx; /* Which area we're operating on */
uint8_t state; /* Which part of the swapping process are we at */
uint8_t use_scratch; /* Are status bytes ever written to scratch? */
+ uint8_t swap_type; /* The type of swap in effect */
uint32_t swap_size; /* Total size of swapped image */
#ifdef MCUBOOT_ENC_IMAGES
uint8_t enckey[2][BOOT_ENC_KEY_SIZE];
@@ -71,6 +72,7 @@
#define BOOT_MAGIC_BAD 2
#define BOOT_MAGIC_UNSET 3
#define BOOT_MAGIC_ANY 4 /* NOTE: control only, not dependent on sector */
+#define BOOT_MAGIC_NOTGOOD 5 /* NOTE: control only, not dependent on sector */
/*
* NOTE: leave BOOT_FLAG_SET equal to one, this is written to flash!
@@ -89,31 +91,42 @@
/**
* End-of-image slot structure.
*
- * 0 1 2 3
- * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * ~ ~
- * ~ Swap status (variable, aligned) ~
- * ~ ~
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | Swap size |
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * ~ padding with erased val (MAX ALIGN - 4) ~
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | Copy done | padding with erased val (MAX ALIGN - 1) ~
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | Image OK | padding with erased val (MAX ALIGN - 1) ~
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * ~ MAGIC (16 octets) ~
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * 0 1 2 3
+ * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ~ ~
+ * ~ Swap status (BOOT_MAX_IMG_SECTORS * min-write-size * 3) ~
+ * ~ ~
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Encryption key 0 (16 octets) [*] |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Encryption key 1 (16 octets) [*] |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Swap size (4 octets) |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Swap type | 0xff padding (7 octets) |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Copy done | 0xff padding (7 octets) |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Image OK | 0xff padding (7 octets) |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | MAGIC (16 octets) |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * [*]: Only present if the encryption option is enabled
+ * (`MCUBOOT_ENC_IMAGES`).
*/
extern const uint32_t boot_img_magic[4];
struct boot_swap_state {
- uint8_t magic; /* One of the BOOT_MAGIC_[...] values. */
- uint8_t copy_done;
- uint8_t image_ok;
+ uint8_t magic; /* One of the BOOT_MAGIC_[...] values. */
+ uint8_t swap_type; /* One of the BOOT_SWAP_TYPE_[...] values. */
+ uint8_t copy_done; /* One of the BOOT_FLAG_[...] values. */
+ uint8_t image_ok; /* One of the BOOT_FLAG_[...] values. */
};
#define BOOT_MAX_IMG_SECTORS MCUBOOT_MAX_IMG_SECTORS
@@ -143,9 +156,6 @@
#define BOOT_STATUS_SOURCE_SCRATCH 1
#define BOOT_STATUS_SOURCE_PRIMARY_SLOT 2
-#define BOOT_FLAG_IMAGE_OK 0
-#define BOOT_FLAG_COPY_DONE 1
-
extern const uint32_t BOOT_MAGIC_SZ;
/**
@@ -180,9 +190,11 @@
int bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig,
size_t slen, uint8_t key_id);
+int boot_magic_compatible_check(uint8_t tbl_val, uint8_t val);
uint32_t boot_trailer_sz(uint8_t min_write_sz);
int boot_status_entries(const struct flash_area *fap);
uint32_t boot_status_off(const struct flash_area *fap);
+uint32_t boot_swap_type_off(const struct flash_area *fap);
int boot_read_swap_state(const struct flash_area *fap,
struct boot_swap_state *state);
int boot_read_swap_state_by_id(int flash_area_id,
@@ -192,6 +204,7 @@
int boot_schedule_test_swap(void);
int boot_write_copy_done(const struct flash_area *fap);
int boot_write_image_ok(const struct flash_area *fap);
+int boot_write_swap_type(const struct flash_area *fap, uint8_t swap_type);
int boot_write_swap_size(const struct flash_area *fap, uint32_t swap_size);
int boot_read_swap_size(uint32_t *swap_size);
#ifdef MCUBOOT_ENC_IMAGES
diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c
index 1240e23..d3aa891 100644
--- a/boot/bootutil/src/loader.c
+++ b/boot/bootutil/src/loader.c
@@ -78,7 +78,7 @@
* ----------------------------------------'
*/
.bst_magic_primary_slot = BOOT_MAGIC_GOOD,
- .bst_magic_scratch = BOOT_MAGIC_ANY,
+ .bst_magic_scratch = BOOT_MAGIC_NOTGOOD,
.bst_copy_done_primary_slot = BOOT_FLAG_SET,
.bst_status_source = BOOT_STATUS_SOURCE_NONE,
},
@@ -93,7 +93,7 @@
* ----------------------------------------'
*/
.bst_magic_primary_slot = BOOT_MAGIC_GOOD,
- .bst_magic_scratch = BOOT_MAGIC_ANY,
+ .bst_magic_scratch = BOOT_MAGIC_NOTGOOD,
.bst_copy_done_primary_slot = BOOT_FLAG_UNSET,
.bst_status_source = BOOT_STATUS_SOURCE_PRIMARY_SLOT,
},
@@ -136,11 +136,13 @@
(sizeof boot_status_tables / sizeof boot_status_tables[0])
#define BOOT_LOG_SWAP_STATE(area, state) \
- BOOT_LOG_INF("%s: magic=%s, copy_done=0x%x, image_ok=0x%x", \
+ BOOT_LOG_INF("%s: magic=%s, swap_type=0x%x, copy_done=0x%x, " \
+ "image_ok=0x%x", \
(area), \
((state)->magic == BOOT_MAGIC_GOOD ? "good" : \
(state)->magic == BOOT_MAGIC_UNSET ? "unset" : \
"bad"), \
+ (state)->swap_type, \
(state)->copy_done, \
(state)->image_ok)
@@ -175,10 +177,10 @@
for (i = 0; i < BOOT_STATUS_TABLES_COUNT; i++) {
table = &boot_status_tables[i];
- if ((table->bst_magic_primary_slot == BOOT_MAGIC_ANY ||
- table->bst_magic_primary_slot == state_primary_slot.magic) &&
- (table->bst_magic_scratch == BOOT_MAGIC_ANY ||
- table->bst_magic_scratch == state_scratch.magic) &&
+ if (boot_magic_compatible_check(table->bst_magic_primary_slot,
+ state_primary_slot.magic) &&
+ boot_magic_compatible_check(table->bst_magic_scratch,
+ state_scratch.magic) &&
(table->bst_copy_done_primary_slot == BOOT_FLAG_ANY ||
table->bst_copy_done_primary_slot == state_primary_slot.copy_done))
{
@@ -196,28 +198,6 @@
return BOOT_STATUS_SOURCE_NONE;
}
-/**
- * Calculates the type of swap that just completed.
- *
- * This is used when a swap is interrupted by an external event. After
- * finishing the swap operation determines what the initial request was.
- */
-static int
-boot_previous_swap_type(void)
-{
- int post_swap_type;
-
- post_swap_type = boot_swap_type();
-
- switch (post_swap_type) {
- case BOOT_SWAP_TYPE_NONE : return BOOT_SWAP_TYPE_PERM;
- case BOOT_SWAP_TYPE_REVERT : return BOOT_SWAP_TYPE_TEST;
- case BOOT_SWAP_TYPE_PANIC : return BOOT_SWAP_TYPE_PANIC;
- }
-
- return BOOT_SWAP_TYPE_FAIL;
-}
-
/*
* Compute the total size of the given image. Includes the size of
* the TLVs.
@@ -541,6 +521,7 @@
boot_read_status(struct boot_status *bs)
{
const struct flash_area *fap;
+ uint32_t off;
int status_loc;
int area_id;
int rc;
@@ -548,6 +529,7 @@
memset(bs, 0, sizeof *bs);
bs->idx = BOOT_STATUS_IDX_0;
bs->state = BOOT_STATUS_STATE_0;
+ bs->swap_type = BOOT_SWAP_TYPE_NONE;
#ifdef MCUBOOT_OVERWRITE_ONLY
/* Overwrite-only doesn't make use of the swap status area. */
@@ -578,6 +560,15 @@
}
rc = boot_read_status_bytes(fap, bs);
+ if (rc == 0) {
+ off = boot_swap_type_off(fap);
+ rc = flash_area_read_is_empty(fap, off, &bs->swap_type,
+ sizeof bs->swap_type);
+ if (rc == 1) {
+ bs->swap_type = BOOT_SWAP_TYPE_NONE;
+ rc = 0;
+ }
+ }
flash_area_close(fap);
@@ -980,6 +971,11 @@
rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_SECONDARY, &swap_state);
assert(rc == 0);
+ if (bs->swap_type != BOOT_SWAP_TYPE_NONE) {
+ rc = boot_write_swap_type(fap, bs->swap_type);
+ assert(rc == 0);
+ }
+
if (swap_state.image_ok == BOOT_FLAG_SET) {
rc = boot_write_image_ok(fap);
assert(rc == 0);
@@ -1001,6 +997,7 @@
return 0;
}
+
#endif
#ifndef MCUBOOT_OVERWRITE_ONLY
@@ -1070,6 +1067,7 @@
uint32_t scratch_trailer_off;
struct boot_swap_state swap_state;
size_t last_sector;
+ bool erase_scratch;
int rc;
/* Calculate offset from start of image area. */
@@ -1106,28 +1104,38 @@
if (bs->state == BOOT_STATUS_STATE_0) {
BOOT_LOG_DBG("erasing scratch area");
- rc = boot_erase_sector(fap_scratch, 0, sz);
- assert(rc == 0);
-
- rc = boot_copy_sector(fap_secondary_slot, fap_scratch,
- img_off, 0, copy_sz);
+ rc = boot_erase_sector(fap_scratch, 0, fap_scratch->fa_size);
assert(rc == 0);
if (bs->idx == BOOT_STATUS_IDX_0) {
- if (bs->use_scratch) {
- boot_status_init(fap_scratch, bs);
- } else {
- /* Prepare the status area... here it is known that the
- * last sector is not being used by the image data so it's
- * safe to erase.
+ /* Write a trailer to the scratch area, even if we don't need the
+ * scratch area for status. We need a temporary place to store the
+ * `swap-type` while we erase the primary trailer.
+ */
+ rc = boot_status_init(fap_scratch, bs);
+ assert(rc == 0);
+
+ if (!bs->use_scratch) {
+ /* Prepare the primary status area... here it is known that the
+ * last sector is not being used by the image data so it's safe
+ * to erase.
*/
rc = boot_erase_trailer_sectors(fap_primary_slot);
assert(rc == 0);
- boot_status_init(fap_primary_slot, bs);
+ rc = boot_status_init(fap_primary_slot, bs);
+ assert(rc == 0);
+
+ /* Erase the temporary trailer from the scratch area. */
+ rc = boot_erase_sector(fap_scratch, 0, fap_scratch->fa_size);
+ assert(rc == 0);
}
}
+ rc = boot_copy_sector(fap_secondary_slot, fap_scratch,
+ img_off, 0, copy_sz);
+ assert(rc == 0);
+
bs->state = BOOT_STATUS_STATE_1;
rc = boot_write_status(bs);
BOOT_STATUS_ASSERT(rc == 0);
@@ -1158,7 +1166,9 @@
rc = boot_erase_sector(fap_primary_slot, img_off, sz);
assert(rc == 0);
- /* NOTE: also copy trailer from scratch (has status info) */
+ /* NOTE: If this is the final sector, we exclude the image trailer from
+ * this copy (copy_sz was truncated earlier).
+ */
rc = boot_copy_sector(fap_scratch, fap_primary_slot,
0, img_off, copy_sz);
assert(rc == 0);
@@ -1181,6 +1191,12 @@
assert(rc == 0);
}
+ if (swap_state.swap_type != BOOT_SWAP_TYPE_NONE) {
+ rc = boot_write_swap_type(fap_primary_slot,
+ swap_state.swap_type);
+ assert(rc == 0);
+ }
+
rc = boot_write_swap_size(fap_primary_slot, bs->swap_size);
assert(rc == 0);
@@ -1191,16 +1207,27 @@
rc = boot_write_enc_key(fap_primary_slot, 1, bs->enckey[1]);
assert(rc == 0);
#endif
-
rc = boot_write_magic(fap_primary_slot);
assert(rc == 0);
}
+ /* If we wrote a trailer to the scratch area, erase it after we persist
+ * a trailer to the primary slot. We do this to prevent mcuboot from
+ * reading a stale status from the scratch area in case of immediate
+ * reset.
+ */
+ erase_scratch = bs->use_scratch;
+ bs->use_scratch = 0;
+
bs->idx++;
bs->state = BOOT_STATUS_STATE_0;
- bs->use_scratch = 0;
rc = boot_write_status(bs);
BOOT_STATUS_ASSERT(rc == 0);
+
+ if (erase_scratch) {
+ rc = boot_erase_sector(fap_scratch, 0, sz);
+ assert(rc == 0);
+ }
}
flash_area_close(fap_primary_slot);
@@ -1235,7 +1262,6 @@
const struct flash_area *fap_primary_slot;
const struct flash_area *fap_secondary_slot;
-
(void)bs;
#if defined(MCUBOOT_OVERWRITE_ONLY_FAST)
@@ -1319,6 +1345,7 @@
}
#endif
+#if !defined(MCUBOOT_OVERWRITE_ONLY)
/**
* Swaps the two images in flash. If a prior copy operation was interrupted
* by a system reset, this function completes that operation.
@@ -1331,7 +1358,6 @@
*
* @return 0 on success; nonzero on failure.
*/
-#if !defined(MCUBOOT_OVERWRITE_ONLY)
static int
boot_swap_image(struct boot_status *bs)
{
@@ -1413,7 +1439,6 @@
}
bs->swap_size = copy_size;
-
} else {
/*
* If a swap was under way, the swap_size should already be present
@@ -1568,12 +1593,9 @@
boot_swap_if_needed(int *out_swap_type)
{
struct boot_status bs;
- int swap_type;
int rc;
- /* Determine if we rebooted in the middle of an image swap
- * operation.
- */
+ /* Determine if we rebooted in the middle of an image swap operation. */
rc = boot_read_status(&bs);
assert(rc == 0);
if (rc != 0) {
@@ -1586,23 +1608,20 @@
/* Should never arrive here, overwrite-only mode has no swap state. */
assert(0);
#else
+ /* Determine the type of swap operation being resumed from the
+ * `swap-type` trailer field.
+ */
rc = boot_swap_image(&bs);
-#endif
assert(rc == 0);
+#endif
- /* NOTE: here we have finished a swap resume. The initial request
- * was either a TEST or PERM swap, which now after the completed
- * swap will be determined to be respectively REVERT (was TEST)
- * or NONE (was PERM).
- */
-
- /* Extrapolate the type of the partial swap. We need this
- * information to know how to mark the swap complete in flash.
- */
- swap_type = boot_previous_swap_type();
} else {
- swap_type = boot_validated_swap_type(&bs);
- switch (swap_type) {
+ if (bs.swap_type == BOOT_SWAP_TYPE_NONE) {
+ bs.swap_type = boot_validated_swap_type(&bs);
+ } else if (boot_validate_slot(BOOT_SECONDARY_SLOT, &bs) != 0) {
+ bs.swap_type = BOOT_SWAP_TYPE_FAIL;
+ }
+ switch (bs.swap_type) {
case BOOT_SWAP_TYPE_TEST:
case BOOT_SWAP_TYPE_PERM:
case BOOT_SWAP_TYPE_REVERT:
@@ -1630,7 +1649,7 @@
assert(rc == 0);
/* Returns fail here to trigger a re-read of the headers. */
- swap_type = BOOT_SWAP_TYPE_FAIL;
+ bs.swap_type = BOOT_SWAP_TYPE_FAIL;
}
}
break;
@@ -1638,7 +1657,7 @@
}
}
- *out_swap_type = swap_type;
+ *out_swap_type = bs.swap_type;
return 0;
}
@@ -1715,7 +1734,8 @@
* swap was finished to avoid a new revert.
*/
if (swap_type == BOOT_SWAP_TYPE_REVERT ||
- swap_type == BOOT_SWAP_TYPE_FAIL) {
+ swap_type == BOOT_SWAP_TYPE_FAIL ||
+ swap_type == BOOT_SWAP_TYPE_PERM) {
#ifndef MCUBOOT_OVERWRITE_ONLY
rc = boot_set_image_ok();
if (rc != 0) {
diff --git a/docs/design.md b/docs/design.md
index 5c3d1b9..7955d64 100644
--- a/docs/design.md
+++ b/docs/design.md
@@ -257,18 +257,27 @@
~ Swap status (BOOT_MAX_IMG_SECTORS * min-write-size * 3) ~
~ ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- | Swap size |
+ | Encryption key 0 (16 octets) [*] |
+ | |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- | 0xff padding (4 octets) |
+ | Encryption key 1 (16 octets) [*] |
+ | |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- | Copy done | 0xff padding (7 octets) ~
+ | Swap size (4 octets) |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- | Image OK | 0xff padding (7 octets) ~
+ | Swap type | 0xff padding (7 octets) |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- ~ MAGIC (16 octets) ~
+ | Copy done | 0xff padding (7 octets) |
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | Image OK | 0xff padding (7 octets) |
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | MAGIC (16 octets) |
+ | |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
```
+[*]: Only present if the encryption option is enabled (`MCUBOOT_ENC_IMAGES`).
+
The offset immediately following such a record represents the start of the next
flash area.
@@ -300,18 +309,34 @@
The factor of min-write-sz is due to the behavior of flash hardware. The factor
of 3 is explained below.
-2. Swap size: When beginning a new swap operation, the total size that needs
+2. Encryption keys: key-encrypting keys (KEKs). These keys are needed for
+ image encryption and decryption. See the
+ [encrypted images](encrypted_images.md) document for more information.
+
+3. Swap size: When beginning a new swap operation, the total size that needs
to be swapped (based on the slot with largest image + tlvs) is written to
this location for easier recovery in case of a reset while performing the
swap.
-3. Copy done: A single byte indicating whether the image in this slot is
+4. Swap type: A single byte indicating the type of swap operation in progress.
+ When mcuboot resumes an interrupted swap, it uses this field to determine
+ the type of operation to perform. This field contains one of the following
+ values:
+
+| Name | Value |
+| ------------------------- | ----- |
+| `BOOT_SWAP_TYPE_TEST` | 2 |
+| `BOOT_SWAP_TYPE_PERM` | 3 |
+| `BOOT_SWAP_TYPE_REVERT` | 4 |
+
+
+5. Copy done: A single byte indicating whether the image in this slot is
complete (0x01=done; 0xff=not done).
-4. Image OK: A single byte indicating whether the image in this slot has been
+6. Image OK: A single byte indicating whether the image in this slot has been
confirmed as good by the user (0x01=confirmed; 0xff=not confirmed).
-5. MAGIC: The following 16 bytes, written in host-byte-order:
+7. MAGIC: The following 16 bytes, written in host-byte-order:
``` c
const uint32_t boot_img_magic[4] = {
@@ -328,6 +353,11 @@
image trailers. When using the term "image trailers" what is meant is the
aggregate information provided by both image slot's trailers.
+### New swaps (non-resumes)
+
+For new swaps, mcuboot must inspect a collection of fields to determine which
+swap operation to perform.
+
The image trailers records are structured around the limitations imposed by
flash hardware. As a consequence, they do not have a very intuitive design, and
it is difficult to get a sense of the state of the device just by looking at the
@@ -404,6 +434,13 @@
marking the image in the primary slot as "OK", to prevent further attempts
to swap.
+### Resumed swaps
+
+If mcuboot determines that it is resuming an interrupted swap (i.e., a reset
+occurred mid-swap), it fully determines the operation to resume by reading the
+`swap type` field from the active trailer. The set of tables in the previous
+section are not necessary in the resume case.
+
## High-Level Operation
With the terms defined, we can now explore the boot loader's operation. First,
@@ -647,14 +684,15 @@
-----------------------------------------------------------------------'
```
-If the swap status region indicates that the images are not contiguous,
-bootutil completes the swap operation that was in progress when the system was
-reset. In other words, it applies the procedure defined in the previous
-section, moving image 1 into the primary slot and image 0 into the secondary
-slot. If the boot status file indicates that an image part is present in the
-scratch area, this part is copied into the correct location by starting at step
-e or step h in the area-swap procedure, depending on whether the part belongs to
-image 0 or image 1.
+If the swap status region indicates that the images are not contiguous, mcuboot
+determines the type of swap operation that was interrupted by reading the `swap
+type` field in the active image trailer, and then resumes the operation. In
+other words, it applies the procedure defined in the previous section, moving
+image 1 into the primary slot and image 0 into the secondary slot. If the boot
+status indicates that an image part is present in the scratch area, this part
+is copied into the correct location by starting at step e or step h in the
+area-swap procedure, depending on whether the part belongs to image 0 or image
+1.
After the swap operation has been completed, the boot loader proceeds as though
it had just been started.
diff --git a/sim/src/image.rs b/sim/src/image.rs
index 92b052e..96637ae 100644
--- a/sim/src/image.rs
+++ b/sim/src/image.rs
@@ -93,8 +93,7 @@
None => return None,
};
- // NOTE: not accounting "swap_size" because it is not used by sim...
- let offset_from_end = c::boot_magic_sz() + c::boot_max_align() * 2;
+ let offset_from_end = c::boot_magic_sz() + c::boot_max_align() * 4;
// Construct a primary image.
let primary = SlotInfo {
@@ -617,7 +616,7 @@
0
};
- self.trailer_sz(align) - (16 + 24 + bias)
+ self.trailer_sz(align) - (16 + 32 + bias)
}
/// This test runs a simple upgrade with no fails in the images, but
@@ -753,8 +752,8 @@
let dev_id = &image.slots[slot].dev_id;
let dev = flash.get_mut(&dev_id).unwrap();
let align = dev.align();
- let off = &image.slots[0].base_off;
- let len = &image.slots[0].len;
+ let off = &image.slots[slot].base_off;
+ let len = &image.slots[slot].len;
let status_off = off + len - self.trailer_sz(align);
// Mark the status area as a bad area
@@ -1198,9 +1197,9 @@
return true;
}
- let offset = slots[slot].trailer_off;
+ let offset = slots[slot].trailer_off + c::boot_max_align();
let dev_id = slots[slot].dev_id;
- let mut copy = vec![0u8; c::boot_magic_sz() + c::boot_max_align() * 2];
+ let mut copy = vec![0u8; c::boot_magic_sz() + c::boot_max_align() * 3];
let mut failed = false;
let dev = flash.get(&dev_id).unwrap();
@@ -1209,12 +1208,12 @@
failed |= match magic {
Some(v) => {
- if v == 1 && ©[16..] != MAGIC.unwrap() {
+ if v == 1 && ©[24..] != MAGIC.unwrap() {
warn!("\"magic\" mismatch at {:#x}", offset);
true
} else if v == 3 {
let expected = [erased_val; 16];
- if ©[16..] != expected {
+ if ©[24..] != expected {
warn!("\"magic\" mismatch at {:#x}", offset);
true
} else {
@@ -1229,7 +1228,7 @@
failed |= match image_ok {
Some(v) => {
- if (v == 1 && copy[8] != v) || (v == 3 && copy[8] != erased_val) {
+ if (v == 1 && copy[16] != v) || (v == 3 && copy[16] != erased_val) {
warn!("\"image_ok\" mismatch at {:#x} v={} val={:#x}", offset, v, copy[8]);
true
} else {
@@ -1241,7 +1240,7 @@
failed |= match copy_done {
Some(v) => {
- if (v == 1 && copy[0] != v) || (v == 3 && copy[0] != erased_val) {
+ if (v == 1 && copy[8] != v) || (v == 3 && copy[8] != erased_val) {
warn!("\"copy_done\" mismatch at {:#x} v={} val={:#x}", offset, v, copy[0]);
true
} else {
@@ -1300,7 +1299,7 @@
/// Write out the magic so that the loader tries doing an upgrade.
pub fn mark_upgrade(flash: &mut SimMultiFlash, slot: &SlotInfo) {
let dev = flash.get_mut(&slot.dev_id).unwrap();
- let offset = slot.trailer_off + c::boot_max_align() * 2;
+ let offset = slot.trailer_off + c::boot_max_align() * 4;
dev.write(offset, MAGIC.unwrap()).unwrap();
}
@@ -1310,7 +1309,7 @@
let dev = flash.get_mut(&slot.dev_id).unwrap();
let mut ok = [dev.erased_val(); 8];
ok[0] = 1u8;
- let off = slot.trailer_off + c::boot_max_align();
+ let off = slot.trailer_off + c::boot_max_align() * 3;
let align = dev.align();
dev.write(off, &ok[..align]).unwrap();
}