Skip to content

Commit f68249e

Browse files
tomchytomchy
committed
bootutil: Unify app_max_size() implementations
Remove redundant application size calculations in favor of a swap-specific function, implemented inside swap_<type>.c. In this way, slot sizes use the same restrictions as image validation. Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>
1 parent 4eba808 commit f68249e

File tree

6 files changed

+197
-190
lines changed

6 files changed

+197
-190
lines changed

‎boot/bootutil/include/bootutil/bootutil_public.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,12 @@ boot_set_next(const struct flash_area *fa, bool active, bool confirm);
295295
/**
296296
* Attempts to load image header from flash; verifies flash header fields.
297297
*
298+
* The selected update method (i.e. swap move) may impose additional restrictions
299+
* on the image size (i.e. due to the presence of the image trailer).
300+
* Such restrictions are not verified by this function.
301+
* These checks are implemented as part of the boot_image_validate(..) that uses
302+
* sizes from the bootutil_max_image_size(..).
303+
*
298304
* @param[in] fa_p flash area pointer
299305
* @param[out] hdr buffer for image header
300306
*

‎boot/bootutil/src/bootutil_misc.c

Lines changed: 8 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
#ifdef MCUBOOT_ENC_IMAGES
4343
#include "bootutil/enc_key.h"
4444
#endif
45+
#if defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET) || \
46+
defined(MCUBOOT_SWAP_USING_SCRATCH)
47+
#include "swap_priv.h"
48+
#endif
4549

4650
#if defined(MCUBOOT_SERIAL_IMG_GRP_SLOT_INFO) || defined(MCUBOOT_SWAP_USING_SCRATCH)
4751
#include "swap_priv.h"
@@ -240,8 +244,7 @@ int boot_header_scramble_off_sz(const struct flash_area *fa, int slot, size_t *o
240244
* status during the swap of the last sector from primary/secondary (which
241245
* is the first swap operation) and thus only requires space for one swap.
242246
*/
243-
static uint32_t
244-
boot_scratch_trailer_sz(uint32_t min_write_sz)
247+
uint32_t boot_scratch_trailer_sz(uint32_t min_write_sz)
245248
{
246249
return boot_status_entry_sz(min_write_sz) + boot_trailer_info_sz();
247250
}
@@ -427,106 +430,17 @@ boot_write_enc_key(const struct flash_area *fap, uint8_t slot,
427430
}
428431
#endif
429432

430-
#ifdef MCUBOOT_SWAP_USING_SCRATCH
431-
size_t
432-
boot_get_first_trailer_sector(struct boot_loader_state *state, size_t slot, size_t trailer_sz)
433-
{
434-
size_t first_trailer_sector = boot_img_num_sectors(state, slot) - 1;
435-
size_t sector_sz = boot_img_sector_size(state, slot, first_trailer_sector);
436-
size_t trailer_sector_sz = sector_sz;
437-
438-
while (trailer_sector_sz < trailer_sz) {
439-
/* Consider that the image trailer may span across sectors of different sizes */
440-
--first_trailer_sector;
441-
sector_sz = boot_img_sector_size(state, slot, first_trailer_sector);
442-
443-
trailer_sector_sz += sector_sz;
444-
}
445-
446-
return first_trailer_sector;
447-
}
448-
449-
/**
450-
* Returns the offset to the end of the first sector of a given slot that holds image trailer data.
451-
*
452-
* @param state Current bootloader's state.
453-
* @param slot The index of the slot to consider.
454-
* @param trailer_sz The size of the trailer, in bytes.
455-
*
456-
* @return The offset to the end of the first sector of the slot that holds image trailer data.
457-
*/
458-
static uint32_t
459-
get_first_trailer_sector_end_off(struct boot_loader_state *state, size_t slot, size_t trailer_sz)
460-
{
461-
size_t first_trailer_sector = boot_get_first_trailer_sector(state, slot, trailer_sz);
462-
463-
return boot_img_sector_off(state, slot, first_trailer_sector) +
464-
boot_img_sector_size(state, slot, first_trailer_sector);
465-
}
466-
#endif /* MCUBOOT_SWAP_USING_SCRATCH */
467-
468433
uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct flash_area *fap)
469434
{
470435
#if defined(MCUBOOT_SINGLE_APPLICATION_SLOT) || \
471436
defined(MCUBOOT_FIRMWARE_LOADER) || \
472437
defined(MCUBOOT_SINGLE_APPLICATION_SLOT_RAM_LOAD)
473438
(void) state;
474439
return boot_status_off(fap);
475-
#elif defined(MCUBOOT_SWAP_USING_SCRATCH)
476-
size_t slot_trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
477-
size_t slot_trailer_off = flash_area_get_size(fap) - slot_trailer_sz;
478-
479-
/* If the trailer doesn't fit in the last sector of the primary or secondary slot, some padding
480-
* might have to be inserted between the end of the firmware image and the beginning of the
481-
* trailer to ensure there is enough space for the trailer in the scratch area when the last
482-
* sector of the secondary will be copied to the scratch area.
483-
*
484-
* The value of the padding depends on the amount of trailer data that is contained in the first
485-
* trailer containing part of the trailer in the primary and secondary slot.
486-
*/
487-
size_t trailer_sector_primary_end_off =
488-
get_first_trailer_sector_end_off(state, BOOT_PRIMARY_SLOT, slot_trailer_sz);
489-
size_t trailer_sector_secondary_end_off =
490-
get_first_trailer_sector_end_off(state, BOOT_SECONDARY_SLOT, slot_trailer_sz);
491-
492-
size_t trailer_sz_in_first_sector;
493-
494-
if (trailer_sector_primary_end_off > trailer_sector_secondary_end_off) {
495-
trailer_sz_in_first_sector = trailer_sector_primary_end_off - slot_trailer_off;
496-
} else {
497-
trailer_sz_in_first_sector = trailer_sector_secondary_end_off - slot_trailer_off;
498-
}
499-
500-
size_t trailer_padding = 0;
501-
size_t scratch_trailer_sz = boot_scratch_trailer_sz(BOOT_WRITE_SZ(state));
502-
503-
if (scratch_trailer_sz > trailer_sz_in_first_sector) {
504-
trailer_padding = scratch_trailer_sz - trailer_sz_in_first_sector;
505-
}
506-
507-
return slot_trailer_off - trailer_padding;
508-
#elif defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET)
440+
#elif defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET) \
441+
|| defined(MCUBOOT_SWAP_USING_SCRATCH)
509442
(void) fap;
510-
511-
/* The slot whose size is used to compute the maximum image size must be the one containing the
512-
* padding required for the swap. */
513-
#ifdef MCUBOOT_SWAP_USING_MOVE
514-
size_t slot = BOOT_PRIMARY_SLOT;
515-
#else
516-
size_t slot = BOOT_SECONDARY_SLOT;
517-
#endif
518-
519-
const struct flash_area *fap_padded_slot = BOOT_IMG_AREA(state, slot);
520-
assert(fap_padded_slot != NULL);
521-
522-
size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
523-
size_t sector_sz = boot_img_sector_size(state, slot, 0);
524-
size_t padding_sz = sector_sz;
525-
526-
/* The trailer size needs to be sector-aligned */
527-
trailer_sz = ALIGN_UP(trailer_sz, sector_sz);
528-
529-
return flash_area_get_size(fap_padded_slot) - trailer_sz - padding_sz;
443+
return app_max_size(state);
530444
#elif defined(MCUBOOT_OVERWRITE_ONLY)
531445
(void) state;
532446
return boot_swap_info_off(fap);

‎boot/bootutil/src/bootutil_priv.h

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -381,18 +381,14 @@ int boot_read_enc_key(const struct flash_area *fap, uint8_t slot,
381381
struct boot_status *bs);
382382
#endif
383383

384-
#ifdef MCUBOOT_SWAP_USING_SCRATCH
385-
/**
386-
* Finds the first sector of a given slot that holds image trailer data.
387-
*
388-
* @param state Current bootloader's state.
389-
* @param slot The index of the slot to consider.
390-
* @param trailer_sz The size of the trailer, in bytes.
391-
*
392-
* @return The index of the first sector of the slot that holds image trailer data.
384+
#if MCUBOOT_SWAP_USING_SCRATCH
385+
/*
386+
* Similar to `boot_trailer_sz` but this function returns the space used to
387+
* store status in the scratch partition. The scratch partition only stores
388+
* status during the swap of the last sector from primary/secondary (which
389+
* is the first swap operation) and thus only requires space for one swap.
393390
*/
394-
size_t
395-
boot_get_first_trailer_sector(struct boot_loader_state *state, size_t slot, size_t trailer_sz);
391+
uint32_t boot_scratch_trailer_sz(uint32_t min_write_sz);
396392
#endif
397393

398394
/**

‎boot/bootutil/src/swap_move.c

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -227,29 +227,6 @@ boot_status_internal_off(const struct boot_status *bs, int elem_sz)
227227
return off;
228228
}
229229

230-
static int app_max_sectors(struct boot_loader_state *state)
231-
{
232-
uint32_t sz = 0;
233-
uint32_t sector_sz;
234-
uint32_t trailer_sz;
235-
uint32_t first_trailer_idx;
236-
237-
sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
238-
trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
239-
/* subtract 1 for swap and at least 1 for trailer */
240-
first_trailer_idx = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - 2;
241-
242-
while (1) {
243-
sz += sector_sz;
244-
if (sz >= trailer_sz) {
245-
break;
246-
}
247-
first_trailer_idx--;
248-
}
249-
250-
return first_trailer_idx;
251-
}
252-
253230
int
254231
boot_slots_compatible(struct boot_loader_state *state)
255232
{
@@ -258,19 +235,16 @@ boot_slots_compatible(struct boot_loader_state *state)
258235
size_t sector_sz_pri = 0;
259236
size_t sector_sz_sec = 0;
260237
size_t i;
261-
size_t num_usable_sectors_pri;
262238

263239
num_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT);
264240
num_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT);
265-
num_usable_sectors_pri = app_max_sectors(state);
266241

267242
if ((num_sectors_pri != num_sectors_sec) &&
268-
(num_sectors_pri != (num_sectors_sec + 1)) &&
269-
(num_usable_sectors_pri != (num_sectors_sec + 1))) {
243+
(num_sectors_pri != (num_sectors_sec + 1))) {
270244
BOOT_LOG_WRN("Cannot upgrade: not a compatible amount of sectors");
271245
BOOT_LOG_DBG("slot0 sectors: %d, slot1 sectors: %d, usable slot0 sectors: %d",
272246
(int)num_sectors_pri, (int)num_sectors_sec,
273-
(int)(num_usable_sectors_pri - 1));
247+
(int)(num_sectors_pri - 1));
274248
return 0;
275249
} else if (num_sectors_pri > BOOT_MAX_IMG_SECTORS) {
276250
BOOT_LOG_WRN("Cannot upgrade: more sectors than allowed");
@@ -280,7 +254,7 @@ boot_slots_compatible(struct boot_loader_state *state)
280254
/* Optimal says primary has one more than secondary. Always. Both have trailers. */
281255
if (num_sectors_pri != (num_sectors_sec + 1)) {
282256
BOOT_LOG_DBG("Non-optimal sector distribution, slot0 has %d usable sectors (%d assigned) "
283-
"but slot1 has %d assigned", (int)num_usable_sectors_pri,
257+
"but slot1 has %d assigned", (int)(num_sectors_pri - 1),
284258
(int)num_sectors_pri, (int)num_sectors_sec);
285259
}
286260

@@ -340,7 +314,6 @@ swap_status_source(struct boot_loader_state *state)
340314
struct boot_swap_state state_primary_slot;
341315
struct boot_swap_state state_secondary_slot;
342316
int rc;
343-
uint8_t source;
344317
uint8_t image_index;
345318

346319
#if (BOOT_IMAGE_NUMBER == 1)
@@ -365,10 +338,8 @@ swap_status_source(struct boot_loader_state *state)
365338
state_primary_slot.copy_done == BOOT_FLAG_UNSET &&
366339
state_secondary_slot.magic != BOOT_MAGIC_GOOD) {
367340

368-
source = BOOT_STATUS_SOURCE_PRIMARY_SLOT;
369-
370341
BOOT_LOG_INF("Boot source: primary slot");
371-
return source;
342+
return BOOT_STATUS_SOURCE_PRIMARY_SLOT;
372343
}
373344

374345
BOOT_LOG_INF("Boot source: none");
@@ -590,11 +561,23 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
590561

591562
int app_max_size(struct boot_loader_state *state)
592563
{
593-
uint32_t sector_sz_primary;
564+
uint32_t available_pri_sz;
565+
uint32_t available_sec_sz;
566+
567+
size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
568+
size_t sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
569+
size_t padding_sz = sector_sz;
594570

595-
sector_sz_primary = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
571+
/* The trailer size needs to be sector-aligned */
572+
trailer_sz = ALIGN_UP(trailer_sz, sector_sz);
573+
574+
/* The slot whose size is used to compute the maximum image size must be the one containing the
575+
* padding required for the swap.
576+
*/
577+
available_pri_sz = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) * sector_sz - trailer_sz - padding_sz;
578+
available_sec_sz = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT) * sector_sz - trailer_sz;
596579

597-
return app_max_sectors(state) * sector_sz_primary;
580+
return (available_pri_sz < available_sec_sz ? available_pri_sz : available_sec_sz);
598581
}
599582

600583
#endif

‎boot/bootutil/src/swap_offset.c

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -302,66 +302,37 @@ uint32_t boot_status_internal_off(const struct boot_status *bs, int elem_sz)
302302
return off;
303303
}
304304

305-
static int app_max_sectors(struct boot_loader_state *state)
306-
{
307-
uint32_t sz = 0;
308-
uint32_t sector_sz;
309-
uint32_t trailer_sz;
310-
uint32_t available_sectors_pri;
311-
uint32_t available_sectors_sec;
312-
uint32_t trailer_sectors = 0;
313-
314-
sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
315-
trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
316-
317-
while (1) {
318-
sz += sector_sz;
319-
++trailer_sectors;
320-
321-
if (sz >= trailer_sz) {
322-
break;
323-
}
324-
}
325-
326-
available_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - trailer_sectors;
327-
available_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT) - 1;
328-
329-
return (available_sectors_pri < available_sectors_sec ? available_sectors_pri : available_sectors_sec);
330-
}
331-
332305
int boot_slots_compatible(struct boot_loader_state *state)
333306
{
334307
size_t num_sectors_pri;
335308
size_t num_sectors_sec;
336309
size_t sector_sz_pri = 0;
337310
size_t sector_sz_sec = 0;
338311
size_t i;
339-
size_t num_usable_sectors;
340312

341313
num_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT);
342314
num_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT);
343-
num_usable_sectors = app_max_sectors(state);
344315

345316
if (num_sectors_pri != num_sectors_sec &&
346-
(num_sectors_pri + 1) != num_sectors_sec &&
347-
num_usable_sectors != (num_sectors_sec - 1)) {
317+
(num_sectors_pri + 1) != num_sectors_sec) {
348318
BOOT_LOG_WRN("Cannot upgrade: not a compatible amount of sectors");
349319
BOOT_LOG_DBG("slot0 sectors: %d, slot1 sectors: %d, usable sectors: %d",
350320
(int)num_sectors_pri, (int)num_sectors_sec,
351-
(int)(num_usable_sectors));
321+
(int)(num_sectors_sec - 1));
352322
return 0;
353323
} else if (num_sectors_pri > BOOT_MAX_IMG_SECTORS) {
354324
BOOT_LOG_WRN("Cannot upgrade: more sectors than allowed");
355325
return 0;
356326
}
357327

358-
if ((num_usable_sectors + 1) != num_sectors_sec) {
328+
/* Optimal says secondary has one more than primary. Always. Both have trailers. */
329+
if ((num_sectors_pri + 1) != num_sectors_sec) {
359330
BOOT_LOG_DBG("Non-optimal sector distribution, slot0 has %d usable sectors "
360-
"but slot1 has %d usable sectors", (int)(num_usable_sectors),
331+
"but slot1 has %d usable sectors", (int)(num_sectors_pri),
361332
((int)num_sectors_sec - 1));
362333
}
363334

364-
for (i = 0; i < num_usable_sectors; i++) {
335+
for (i = 0; i < (num_sectors_sec - 1); i++) {
365336
sector_sz_pri = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, i);
366337
sector_sz_sec = boot_img_sector_size(state, BOOT_SECONDARY_SLOT, i);
367338

@@ -417,7 +388,6 @@ int swap_status_source(struct boot_loader_state *state)
417388
struct boot_swap_state state_primary_slot;
418389
struct boot_swap_state state_secondary_slot;
419390
int rc;
420-
uint8_t source;
421391
uint8_t image_index;
422392

423393
#if (BOOT_IMAGE_NUMBER == 1)
@@ -439,10 +409,8 @@ int swap_status_source(struct boot_loader_state *state)
439409
state_primary_slot.copy_done == BOOT_FLAG_UNSET &&
440410
state_secondary_slot.magic != BOOT_MAGIC_GOOD) {
441411

442-
source = BOOT_STATUS_SOURCE_PRIMARY_SLOT;
443-
444412
BOOT_LOG_INF("Boot source: primary slot");
445-
return source;
413+
return BOOT_STATUS_SOURCE_PRIMARY_SLOT;
446414
}
447415

448416
BOOT_LOG_INF("Boot source: none");
@@ -729,11 +697,23 @@ void swap_run(struct boot_loader_state *state, struct boot_status *bs,
729697

730698
int app_max_size(struct boot_loader_state *state)
731699
{
732-
uint32_t sector_sz_primary;
700+
uint32_t available_pri_sz;
701+
uint32_t available_sec_sz;
702+
703+
size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
704+
size_t sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
705+
size_t padding_sz = sector_sz;
706+
707+
/* The trailer size needs to be sector-aligned */
708+
trailer_sz = ALIGN_UP(trailer_sz, sector_sz);
733709

734-
sector_sz_primary = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
710+
/* The slot whose size is used to compute the maximum image size must be the one containing the
711+
* padding required for the swap.
712+
*/
713+
available_pri_sz = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) * sector_sz - trailer_sz;
714+
available_sec_sz = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT) * sector_sz - trailer_sz - padding_sz;
735715

736-
return app_max_sectors(state) * sector_sz_primary;
716+
return (available_pri_sz < available_sec_sz ? available_pri_sz : available_sec_sz);
737717
}
738718

739719
/* Compute the total size of the given image. Includes the size of the TLVs. */

0 commit comments

Comments
 (0)