Skip to content

Commit 4fb7af0

Browse files
taltenbachtaltenbach
committed
bootutil: swap-move: Avoid rewriting the secondary trailer on revert
When using the swap-move strategy, at the very beginning of the revert process, the secondary trailer was rewritten to make the revert look like a permanent upgrade in case an unfortunate reset occurs when rewriting the primary trailer. This was possible because the assumption was that no sector contained both part of the firmware and part of the trailer. To relax this assumption, it is necessary to avoid having to rewrite the secondary trailer at the start of the revert process, since that could also erase firmware data. The solution chosen is to rewrite the secondary trailer at the end of the upgrade process, just after that trailer is erased and to take advantage of the unused 'copy_done' flag in the secondary trailer to indicate that an upgrade has been performed and that a revert has to be started or resumed if the primary image is not confirmed. Signed-off-by: Thomas Altenbach <thomas.altenbach@legrand.com>
1 parent 6f329ba commit 4fb7af0

File tree

2 files changed

+50
-66
lines changed

2 files changed

+50
-66
lines changed

‎boot/bootutil/src/bootutil_public.c

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ struct boot_swap_table {
8484
uint8_t image_ok_primary_slot;
8585
uint8_t image_ok_secondary_slot;
8686
uint8_t copy_done_primary_slot;
87-
#if defined(MCUBOOT_SWAP_USING_OFFSET)
87+
#if defined(MCUBOOT_SWAP_USING_OFFSET) || defined(MCUBOOT_SWAP_USING_MOVE)
8888
uint8_t copy_done_secondary_slot;
89-
#endif
89+
#endif /* MCUBOOT_SWAP_USING_OFFSET || MCUBOOT_SWAP_USING_MOVE */
9090

9191
uint8_t swap_type;
9292
};
@@ -103,26 +103,35 @@ struct boot_swap_table {
103103
* the bootloader, as in starting/finishing a swap operation.
104104
*/
105105
static const struct boot_swap_table boot_swap_tables[] = {
106-
#if defined(MCUBOOT_SWAP_USING_OFFSET)
106+
#if defined(MCUBOOT_SWAP_USING_OFFSET) || defined(MCUBOOT_SWAP_USING_MOVE)
107107
{
108-
.magic_primary_slot = BOOT_MAGIC_ANY,
108+
.magic_primary_slot = BOOT_MAGIC_NOTGOOD,
109109
.magic_secondary_slot = BOOT_MAGIC_GOOD,
110110
.image_ok_primary_slot = BOOT_FLAG_ANY,
111111
.image_ok_secondary_slot = BOOT_FLAG_UNSET,
112112
.copy_done_primary_slot = BOOT_FLAG_ANY,
113113
.copy_done_secondary_slot = BOOT_FLAG_SET,
114114
.swap_type = BOOT_SWAP_TYPE_REVERT,
115115
},
116-
#endif
116+
{
117+
.magic_primary_slot = BOOT_MAGIC_GOOD,
118+
.magic_secondary_slot = BOOT_MAGIC_GOOD,
119+
.image_ok_primary_slot = BOOT_FLAG_UNSET,
120+
.image_ok_secondary_slot = BOOT_FLAG_UNSET,
121+
.copy_done_primary_slot = BOOT_FLAG_ANY,
122+
.copy_done_secondary_slot = BOOT_FLAG_SET,
123+
.swap_type = BOOT_SWAP_TYPE_REVERT,
124+
},
125+
#endif /* MCUBOOT_SWAP_USING_OFFSET || MCUBOOT_SWAP_USING_MOVE */
117126
{
118127
.magic_primary_slot = BOOT_MAGIC_ANY,
119128
.magic_secondary_slot = BOOT_MAGIC_GOOD,
120129
.image_ok_primary_slot = BOOT_FLAG_ANY,
121130
.image_ok_secondary_slot = BOOT_FLAG_UNSET,
122131
.copy_done_primary_slot = BOOT_FLAG_ANY,
123-
#if defined(MCUBOOT_SWAP_USING_OFFSET)
124-
.copy_done_secondary_slot = BOOT_FLAG_ANY,
125-
#endif
132+
#if defined(MCUBOOT_SWAP_USING_OFFSET) || defined(MCUBOOT_SWAP_USING_MOVE)
133+
.copy_done_secondary_slot = BOOT_FLAG_UNSET,
134+
#endif /* MCUBOOT_SWAP_USING_OFFSET || MCUBOOT_SWAP_USING_MOVE */
126135
.swap_type = BOOT_SWAP_TYPE_TEST,
127136
},
128137
{
@@ -131,9 +140,9 @@ static const struct boot_swap_table boot_swap_tables[] = {
131140
.image_ok_primary_slot = BOOT_FLAG_ANY,
132141
.image_ok_secondary_slot = BOOT_FLAG_SET,
133142
.copy_done_primary_slot = BOOT_FLAG_ANY,
134-
#if defined(MCUBOOT_SWAP_USING_OFFSET)
135-
.copy_done_secondary_slot = BOOT_FLAG_ANY,
136-
#endif
143+
#if defined(MCUBOOT_SWAP_USING_OFFSET) || defined(MCUBOOT_SWAP_USING_MOVE)
144+
.copy_done_secondary_slot = BOOT_FLAG_UNSET,
145+
#endif /* MCUBOOT_SWAP_USING_OFFSET || MCUBOOT_SWAP_USING_MOVE */
137146
.swap_type = BOOT_SWAP_TYPE_PERM,
138147
},
139148
{
@@ -142,9 +151,9 @@ static const struct boot_swap_table boot_swap_tables[] = {
142151
.image_ok_primary_slot = BOOT_FLAG_UNSET,
143152
.image_ok_secondary_slot = BOOT_FLAG_ANY,
144153
.copy_done_primary_slot = BOOT_FLAG_SET,
145-
#if defined(MCUBOOT_SWAP_USING_OFFSET)
154+
#if defined(MCUBOOT_SWAP_USING_OFFSET) || defined(MCUBOOT_SWAP_USING_MOVE)
146155
.copy_done_secondary_slot = BOOT_FLAG_ANY,
147-
#endif
156+
#endif /* MCUBOOT_SWAP_USING_OFFSET || MCUBOOT_SWAP_USING_MOVE */
148157
.swap_type = BOOT_SWAP_TYPE_REVERT,
149158
},
150159
};
@@ -463,10 +472,10 @@ boot_swap_type_multi(int image_index)
463472
table->image_ok_secondary_slot == secondary_slot.image_ok) &&
464473
(table->copy_done_primary_slot == BOOT_FLAG_ANY ||
465474
table->copy_done_primary_slot == primary_slot.copy_done)
466-
#if defined(MCUBOOT_SWAP_USING_OFFSET)
475+
#if defined(MCUBOOT_SWAP_USING_OFFSET) || defined(MCUBOOT_SWAP_USING_MOVE)
467476
&& (table->copy_done_secondary_slot == BOOT_FLAG_ANY ||
468477
table->copy_done_secondary_slot == secondary_slot.copy_done)
469-
#endif
478+
#endif /* MCUBOOT_SWAP_USING_OFFSET || MCUBOOT_SWAP_USING_MOVE */
470479
) {
471480
BOOT_LOG_INF("Image index: %d, Swap type: %s", image_index,
472481
table->swap_type == BOOT_SWAP_TYPE_TEST ? "test" :

‎boot/bootutil/src/swap_move.c

Lines changed: 26 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,32 @@ boot_swap_sectors(size_t idx, size_t last_idx, uint32_t sz, struct boot_loader_s
477477
rc = swap_scramble_trailer_sectors(state, fap_sec);
478478
assert(rc == 0);
479479

480+
/* When starting a revert the swap status of the primary slot is erased then
481+
* re-initialized. If a reset occurs after the erasure but before the re-initialization
482+
* is complete, there has to be, somewhere, a flag indicating a revert is in progress.
483+
* Otherwise, the bootloader wouldn't be able to resume the revert operation.
484+
*
485+
* Initially, the swap-move algorithm was assuming the trailers were stored in dedicated
486+
* sectors and it was therefore possible to rewrite the secondary trailer before
487+
* starting the revert process, to make the revert look like a permanent upgrade in case
488+
* an unfortunate reset occurs during the rewriting of the primary trailer.
489+
*
490+
* However, considering now the first trailer sector could also hold firmware data, this
491+
* trick is no more possible since it could potentially erase part of the firmware image
492+
* to be restored. A solution is to rewrite here the secondary trailer with the
493+
* 'copy_done' flag set, meaning after an upgrade the secondary trailer is no more
494+
* erased. The bootloader will consider a revert must be started or resumed if the
495+
* primary image is not confirmed and the 'copy_done' flag is set in the secondary
496+
* trailer.
497+
*/
498+
if (bs->swap_type != BOOT_SWAP_TYPE_REVERT) {
499+
rc = boot_write_copy_done(fap_sec);
500+
assert(rc == 0);
501+
502+
rc = boot_write_magic(fap_sec);
503+
assert(rc == 0);
504+
}
505+
480506
size_t first_trailer_sector_pri =
481507
boot_get_first_trailer_sector(state, BOOT_PRIMARY_SLOT);
482508
size_t first_trailer_sector_sec =
@@ -516,55 +542,6 @@ boot_swap_sectors(size_t idx, size_t last_idx, uint32_t sz, struct boot_loader_s
516542
}
517543
}
518544

519-
/*
520-
* When starting a revert the swap status exists in the primary slot, and
521-
* the status in the secondary slot is erased. To start the swap, the status
522-
* area in the primary slot must be re-initialized; if during the small
523-
* window of time between re-initializing it and writing the first metadata
524-
* a reset happens, the swap process is broken and cannot be resumed.
525-
*
526-
* This function handles the issue by making the revert look like a permanent
527-
* upgrade (by initializing the secondary slot).
528-
*/
529-
void
530-
fixup_revert(const struct boot_loader_state *state, struct boot_status *bs,
531-
const struct flash_area *fap_sec)
532-
{
533-
struct boot_swap_state swap_state;
534-
int rc;
535-
536-
#if (BOOT_IMAGE_NUMBER == 1)
537-
(void)state;
538-
#endif
539-
540-
/* No fixup required */
541-
if (bs->swap_type != BOOT_SWAP_TYPE_REVERT ||
542-
bs->op != BOOT_STATUS_OP_MOVE ||
543-
bs->idx != BOOT_STATUS_IDX_0) {
544-
return;
545-
}
546-
547-
rc = boot_read_swap_state(fap_sec, &swap_state);
548-
assert(rc == 0);
549-
550-
BOOT_LOG_SWAP_STATE("Secondary image", &swap_state);
551-
552-
if (swap_state.magic == BOOT_MAGIC_UNSET) {
553-
/* Remove trailer and prepare area for write on devices requiring erase */
554-
rc = swap_scramble_trailer_sectors(state, fap_sec);
555-
assert(rc == 0);
556-
557-
rc = boot_write_image_ok(fap_sec);
558-
assert(rc == 0);
559-
560-
rc = boot_write_swap_size(fap_sec, bs->swap_size);
561-
assert(rc == 0);
562-
563-
rc = boot_write_magic(fap_sec);
564-
assert(rc == 0);
565-
}
566-
}
567-
568545
void
569546
swap_run(struct boot_loader_state *state, struct boot_status *bs,
570547
uint32_t copy_size)
@@ -615,8 +592,6 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
615592
fap_sec = BOOT_IMG_AREA(state, BOOT_SECONDARY_SLOT);
616593
assert(fap_sec != NULL);
617594

618-
fixup_revert(state, bs, fap_sec);
619-
620595
if (bs->op == BOOT_STATUS_OP_MOVE) {
621596
idx = last_idx;
622597
while (idx > 0) {

0 commit comments

Comments
 (0)