Skip to content

boot: bootutil: Fix max image size computation for swap-move and swap-offset #2283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

boot: bootutil: Fix max image size computation for swap-move and swap-offset #2283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd9e33c
boot: bootutil: Fix max image size computation for swap-move/swap-offset
taltenbach Apr 27, 2025
3589fb9
boot: bootutil: swap-offset: Fix image size check during validation
taltenbach Apr 27, 2025
cfa8e42
sim: Fix largest image computation for swap-move and swap-offset
taltenbach Mar 30, 2025
52ba928
sim: Use largest image for all upgrade strategies
taltenbach Apr 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions boot/bootutil/src/bootutil_misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,18 +476,27 @@ uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct f

return slot_trailer_off - trailer_padding;
#elif defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET)
(void) state;
(void) fap;

struct flash_sector sector;
/* get the last sector offset */
int rc = flash_area_get_sector(fap, boot_status_off(fap), &sector);
if (rc) {
BOOT_LOG_ERR("Unable to determine flash sector of the image trailer");
return 0; /* Returning of zero here should cause any check which uses
* this value to fail.
*/
}
return flash_sector_get_off(&sector);
/* The slot whose size is used to compute the maximum image size must be the one containing the
* padding required for the swap. */
#ifdef MCUBOOT_SWAP_USING_MOVE
size_t slot = BOOT_PRIMARY_SLOT;
#else
size_t slot = BOOT_SECONDARY_SLOT;
#endif

const struct flash_area *fap_padded_slot = BOOT_IMG_AREA(state, slot);
assert(fap_padded_slot != NULL);

size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
size_t sector_sz = boot_img_sector_size(state, slot, 0);
size_t padding_sz = sector_sz;

/* The trailer size needs to be sector-aligned */
trailer_sz = ALIGN_UP(trailer_sz, sector_sz);

return flash_area_get_size(fap_padded_slot) - trailer_sz - padding_sz;
#elif defined(MCUBOOT_OVERWRITE_ONLY)
(void) state;
return boot_swap_info_off(fap);
Expand Down
9 changes: 8 additions & 1 deletion boot/bootutil/src/image_validate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ bootutil_img_validate(struct boot_loader_state *state,
uint32_t off;
uint16_t len;
uint16_t type;
uint32_t img_sz;
#ifdef EXPECTED_SIG_TLV
FIH_DECLARE(valid_signature, FIH_FAILURE);
#ifndef MCUBOOT_BUILTIN_KEY
Expand Down Expand Up @@ -555,7 +556,13 @@ bootutil_img_validate(struct boot_loader_state *state,
goto out;
}

if (it.tlv_end > bootutil_max_image_size(state, fap)) {
#ifdef MCUBOOT_SWAP_USING_OFFSET
img_sz = it.tlv_end - it.start_off;
#else
img_sz = it.tlv_end;
#endif

if (img_sz > bootutil_max_image_size(state, fap)) {
rc = -1;
goto out;
}
Expand Down
138 changes: 74 additions & 64 deletions sim/src/image.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,21 +234,21 @@ impl ImagesBuilder {

let (primaries,upgrades) = if img_manipulation == ImageManipulation::CorruptHigherVersionImage && !higher_version_corrupted {
higher_version_corrupted = true;
let prim = install_image(&mut flash, &self.areadesc, &slots[0],
maximal(42784), &ram, &*dep, ImageManipulation::None, Some(0), false);
let prim = install_image(&mut flash, &self.areadesc, &slots, 0,
maximal(42784), &ram, &*dep, ImageManipulation::None, Some(0));
let upgr = match deps.depends[image_num] {
DepType::NoUpgrade => install_no_image(),
_ => install_image(&mut flash, &self.areadesc, &slots[1],
maximal(46928), &ram, &*dep, ImageManipulation::BadSignature, Some(0), true)
_ => install_image(&mut flash, &self.areadesc, &slots, 1,
maximal(46928), &ram, &*dep, ImageManipulation::BadSignature, Some(0))
};
(prim, upgr)
} else {
let prim = install_image(&mut flash, &self.areadesc, &slots[0],
maximal(42784), &ram, &*dep, img_manipulation, Some(0), false);
let prim = install_image(&mut flash, &self.areadesc, &slots, 0,
maximal(42784), &ram, &*dep, img_manipulation, Some(0));
let upgr = match deps.depends[image_num] {
DepType::NoUpgrade => install_no_image(),
_ => install_image(&mut flash, &self.areadesc, &slots[1],
maximal(46928), &ram, &*dep, img_manipulation, Some(0), true)
_ => install_image(&mut flash, &self.areadesc, &slots, 1,
maximal(46928), &ram, &*dep, img_manipulation, Some(0))
};
(prim, upgr)
};
Expand Down Expand Up @@ -298,10 +298,10 @@ impl ImagesBuilder {
let ram = self.ram.clone(); // TODO: Avoid this clone.
let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
let primaries = install_image(&mut bad_flash, &self.areadesc, &slots[0],
maximal(32784), &ram, &dep, ImageManipulation::None, Some(0), false);
let upgrades = install_image(&mut bad_flash, &self.areadesc, &slots[1],
maximal(41928), &ram, &dep, ImageManipulation::BadSignature, Some(0), true);
let primaries = install_image(&mut bad_flash, &self.areadesc, &slots, 0,
maximal(32784), &ram, &dep, ImageManipulation::None, Some(0));
let upgrades = install_image(&mut bad_flash, &self.areadesc, &slots, 1,
maximal(41928), &ram, &dep, ImageManipulation::BadSignature, Some(0));
OneImage {
slots,
primaries,
Expand All @@ -321,10 +321,10 @@ impl ImagesBuilder {
let ram = self.ram.clone(); // TODO: Avoid this clone.
let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
let primaries = install_image(&mut bad_flash, &self.areadesc, &slots[0],
maximal(32784), &ram, &dep, ImageManipulation::None, Some(0), false);
let upgrades = install_image(&mut bad_flash, &self.areadesc, &slots[1],
ImageSize::Oversized, &ram, &dep, ImageManipulation::None, Some(0), true);
let primaries = install_image(&mut bad_flash, &self.areadesc, &slots, 0,
maximal(32784), &ram, &dep, ImageManipulation::None, Some(0));
let upgrades = install_image(&mut bad_flash, &self.areadesc, &slots, 1,
ImageSize::Oversized, &ram, &dep, ImageManipulation::None, Some(0));
OneImage {
slots,
primaries,
Expand All @@ -344,8 +344,8 @@ impl ImagesBuilder {
let ram = self.ram.clone(); // TODO: Avoid this clone.
let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
let primaries = install_image(&mut flash, &self.areadesc, &slots[0],
maximal(32784), &ram, &dep,ImageManipulation::None, Some(0), false);
let primaries = install_image(&mut flash, &self.areadesc, &slots, 0,
maximal(32784), &ram, &dep,ImageManipulation::None, Some(0));
let upgrades = install_no_image();
OneImage {
slots,
Expand All @@ -367,8 +367,8 @@ impl ImagesBuilder {
let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
let primaries = install_no_image();
let upgrades = install_image(&mut flash, &self.areadesc, &slots[1],
maximal(32784), &ram, &dep, ImageManipulation::None, Some(0), true);
let upgrades = install_image(&mut flash, &self.areadesc, &slots, 1,
maximal(32784), &ram, &dep, ImageManipulation::None, Some(0));
OneImage {
slots,
primaries,
Expand All @@ -389,8 +389,8 @@ impl ImagesBuilder {
let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
let primaries = install_no_image();
let upgrades = install_image(&mut flash, &self.areadesc, &slots[1],
ImageSize::Oversized, &ram, &dep, ImageManipulation::None, Some(0), true);
let upgrades = install_image(&mut flash, &self.areadesc, &slots, 1,
ImageSize::Oversized, &ram, &dep, ImageManipulation::None, Some(0));
OneImage {
slots,
primaries,
Expand All @@ -411,10 +411,10 @@ impl ImagesBuilder {
let ram = self.ram.clone(); // TODO: Avoid this clone.
let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
let primaries = install_image(&mut flash, &self.areadesc, &slots[0],
maximal(32784), &ram, &dep, ImageManipulation::None, security_cnt, false);
let upgrades = install_image(&mut flash, &self.areadesc, &slots[1],
maximal(41928), &ram, &dep, ImageManipulation::None, security_cnt.map(|v| v + 1), true);
let primaries = install_image(&mut flash, &self.areadesc, &slots, 0,
maximal(32784), &ram, &dep, ImageManipulation::None, security_cnt);
let upgrades = install_image(&mut flash, &self.areadesc, &slots, 1,
maximal(41928), &ram, &dep, ImageManipulation::None, security_cnt.map(|v| v + 1));
OneImage {
slots,
primaries,
Expand Down Expand Up @@ -1747,6 +1747,7 @@ fn show_flash(flash: &dyn Flash) {
#[derive(Debug)]
enum ImageSize {
/// Make the image the specified given size.
#[allow(dead_code)]
Given(usize),
/// Make the image as large as it can be for the partition/device.
Largest,
Expand Down Expand Up @@ -1819,19 +1820,52 @@ fn image_largest_trailer(dev: &dyn Flash, areadesc: &AreaDesc, slot: &SlotInfo)
trailer
}

// Computes the padding required in the primary or secondary slot to be able to perform an upgrade.
// This is needed only for the swap-move and swap-offset upgrade strategies.
fn required_slot_padding(dev: &dyn Flash) -> usize {
let mut required_padding = 0;

if Caps::SwapUsingMove.present() || Caps::SwapUsingOffset.present() {
// Assumes equally-sized sectors
let sector_size = dev.sector_iter().next().unwrap().size;

required_padding = sector_size;
};

required_padding
}

// Computes the largest possible firmware image size, not including the header and TLV area.
fn compute_largest_image_size(dev: &dyn Flash, areadesc: &AreaDesc, slots: &[SlotInfo],
slot_ind: usize, hdr_size: usize, tlv: &dyn ManifestGen) -> usize {
let slot_len = if Caps::SwapUsingOffset.present() {
slots[1].len
} else {
slots[0].len
};

let trailer = image_largest_trailer(dev, areadesc, &slots[slot_ind]);
let padding = required_slot_padding(dev);
let tlv_len = tlv.estimate_size();
info!("slot: 0x{:x}, HDR: 0x{:x}, trailer: 0x{:x}, tlv_len: 0x{:x}, padding: 0x{:x}",
slot_len, hdr_size, trailer, tlv_len, padding);

slot_len - hdr_size - trailer - tlv_len - padding
}

/// Install a "program" into the given image. This fakes the image header, or at least all of the
/// fields used by the given code. Returns a copy of the image that was written.
fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slot: &SlotInfo, len: ImageSize,
ram: &RamData,
deps: &dyn Depender, img_manipulation: ImageManipulation, security_counter:Option<u32>, secondary_slot:bool) -> ImageData {
fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slots: &[SlotInfo],
slot_ind: usize, len: ImageSize, ram: &RamData,
deps: &dyn Depender, img_manipulation: ImageManipulation, security_counter:Option<u32>) -> ImageData {
let slot = &slots[slot_ind];
let mut offset = slot.base_off;
let slot_len = slot.len;
let dev_id = slot.dev_id;
let dev = flash.get_mut(&dev_id).unwrap();

let mut tlv: Box<dyn ManifestGen> = Box::new(make_tlv());

if Caps::SwapUsingOffset.present() && secondary_slot {
if Caps::SwapUsingOffset.present() && slot_ind == 1 {
let sector_size = dev.sector_iter().next().unwrap().size as usize;
offset += sector_size;
}
Expand Down Expand Up @@ -1863,30 +1897,13 @@ fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slot: &SlotInfo

let len = match len {
ImageSize::Given(size) => size,
ImageSize::Largest => {
let trailer = image_largest_trailer(dev, &areadesc, &slot);
let tlv_len = tlv.estimate_size();
info!("slot: 0x{:x}, HDR: 0x{:x}, trailer: 0x{:x}",
slot_len, HDR_SIZE, trailer);
slot_len - HDR_SIZE - trailer - tlv_len
},
ImageSize::Largest => compute_largest_image_size(dev, areadesc, slots, slot_ind,
HDR_SIZE, tlv.as_ref()),
ImageSize::Oversized => {
let trailer = image_largest_trailer(dev, &areadesc, &slot);
let tlv_len = tlv.estimate_size();
let mut sector_offset = 0;

if Caps::SwapUsingOffset.present() && secondary_slot {
// This accounts for when both slots have the same size, it will not work where
// the second slot is one sector larger than the primary
sector_offset = dev.sector_iter().next().unwrap().size as usize;
}

info!("slot: 0x{:x}, HDR: 0x{:x}, trailer: 0x{:x}",
slot_len, HDR_SIZE, trailer);

slot_len - HDR_SIZE - trailer - tlv_len - sector_offset + dev.align()
let largest_img_sz = compute_largest_image_size(dev, areadesc, slots, slot_ind,
HDR_SIZE, tlv.as_ref());
largest_img_sz + dev.align()
}

};

// Generate a boot header. Note that the size doesn't include the header.
Expand Down Expand Up @@ -1995,7 +2012,7 @@ fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slot: &SlotInfo

enc_copy = Some(enc);

dev.erase(offset, slot_len).unwrap();
dev.erase(offset, slot.len).unwrap();
} else {
enc_copy = None;
}
Expand All @@ -2020,7 +2037,7 @@ fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slot: &SlotInfo
let enc_copy: Option<Vec<u8>>;

if is_encrypted {
dev.erase(offset, slot_len).unwrap();
dev.erase(offset, slot.len).unwrap();

dev.write(offset, &encbuf).unwrap();

Expand Down Expand Up @@ -2394,15 +2411,8 @@ trait AsRaw : Sized {

/// Determine whether it makes sense to test this configuration with a maximally-sized image.
/// Returns an ImageSize representing the best size to test, possibly just with the given size.
fn maximal(size: usize) -> ImageSize {
if Caps::OverwriteUpgrade.present() ||
Caps::SwapUsingOffset.present() ||
Caps::SwapUsingMove.present()
{
ImageSize::Given(size)
} else {
ImageSize::Largest
}
fn maximal(_size: usize) -> ImageSize {
ImageSize::Largest
}

pub fn show_sizes() {
Expand Down
Loading