Better fix for elf squashing (#341)
Don't squash into occupied areas, and don't squash metadata blocks
This reverts commit 0d160860227c83133502b8e1c504313e40ae9124
* Catch invalid cycles in block loops (e.g. lollipop block loops)
* Fix XIP SRAM only binary support
diff --git a/bintool/bintool.cpp b/bintool/bintool.cpp
index 9d54431..6c11693 100644
--- a/bintool/bintool.cpp
+++ b/bintool/bintool.cpp
@@ -6,6 +6,7 @@
#include <random>
#include <cinttypes>
#include <tuple>
+#include <set>
#include "boot/picobin.h"
#include <map>
@@ -255,6 +256,8 @@
if (psize == 0) continue;
if (paddr >= model->sram_start() && paddr < model->sram_striped_end()) {
highest_ram_address = std::max(paddr + psize, highest_ram_address);
+ } else if (paddr >= model->xip_sram_start() && paddr < model->xip_sram_end()) {
+ highest_ram_address = std::max(paddr + psize, highest_ram_address);
} else if (paddr >= model->flash_start() && paddr < model->flash_end()) {
highest_flash_address = std::max(paddr + psize, highest_flash_address);
}
@@ -283,44 +286,11 @@
} else {
DEBUG_LOG("There is already a block loop\n");
if (set_others_ignored) set_block_ignored(elf, first_block->physical_addr);
- uint32_t next_block_addr = first_block->physical_addr + first_block->next_block_rel;
- while (true) {
- auto segment = elf->segment_from_physical_address(next_block_addr);
- if (segment == nullptr) {
- fail(ERROR_NOT_POSSIBLE, "The ELF file does not contain the next block address %x", next_block_addr);
- }
- auto data = elf->content(*segment);
- auto offset = next_block_addr - segment->physical_address();
- std::vector<uint32_t> words = lsb_bytes_to_words(data.begin() + offset, data.end());
- if (words.front() != PICOBIN_BLOCK_MARKER_START) {
- fail(ERROR_UNKNOWN, "Block loop is not valid - no block found at %08x\n", (int)(next_block_addr));
- }
- words.erase(words.begin());
- DEBUG_LOG("Checking block at %x\n", next_block_addr);
- for(auto next_item = words.begin(); next_item < words.end(); ) {
- unsigned int size = item::decode_size(*next_item);
- if ((uint8_t)*next_item == PICOBIN_BLOCK_ITEM_2BS_LAST) {
- if (size == next_item - words.begin()) {
- if (next_item < words.end() && next_item[2] == PICOBIN_BLOCK_MARKER_END) {
- DEBUG_LOG("is a valid block\n");
- new_first_block = block::parse(next_block_addr, next_item + 1, words.begin(), words.begin() + size);
- break;
- }
- }
- } else {
- next_item += size;
- }
- }
- if (new_first_block->physical_addr + new_first_block->next_block_rel == first_block->physical_addr) {
- DEBUG_LOG("Found last block in block loop\n");
- break;
- } else {
- DEBUG_LOG("Continue looping\n");
- if (set_others_ignored) set_block_ignored(elf, new_first_block->physical_addr);
- next_block_addr = new_first_block->physical_addr + new_first_block->next_block_rel;
- new_first_block.reset();
- }
+ auto all_blocks = get_all_blocks(elf, first_block);
+ for (auto &block : all_blocks) {
+ if (set_others_ignored) set_block_ignored(elf, block->physical_addr);
}
+ new_first_block = std::move(all_blocks.back());
set_next_block(elf, new_first_block, highest_address);
new_block_addr = new_first_block->physical_addr + new_first_block->next_block_rel;
loop_start_rel = first_block->physical_addr - new_block_addr;
@@ -362,8 +332,65 @@
}
+std::vector<std::unique_ptr<block>> get_all_blocks(elf_file *elf, std::unique_ptr<block> &first_block) {
+ uint32_t next_block_addr = first_block->physical_addr + first_block->next_block_rel;
+ std::set<uint32_t> next_block_addrs = {next_block_addr};
+ std::vector<std::unique_ptr<block>> all_blocks;
+ while (true) {
+ auto segment = elf->segment_from_physical_address(next_block_addr);
+ if (segment == nullptr) {
+ fail(ERROR_NOT_POSSIBLE, "The ELF file does not contain the next block address %x", next_block_addr);
+ }
+ auto data = elf->content(*segment);
+ auto offset = next_block_addr - segment->physical_address();
+ std::vector<uint32_t> words = lsb_bytes_to_words(data.begin() + offset, data.end());
+ if (words.front() != PICOBIN_BLOCK_MARKER_START) {
+ fail(ERROR_UNKNOWN, "Block loop is not valid - no block found at %08x\n", (int)(next_block_addr));
+ }
+ words.erase(words.begin());
+ DEBUG_LOG("Checking block at %x\n", next_block_addr);
+ std::unique_ptr<block> new_first_block;
+ for(auto next_item = words.begin(); next_item < words.end(); ) {
+ unsigned int size = item::decode_size(*next_item);
+ if ((uint8_t)*next_item == PICOBIN_BLOCK_ITEM_2BS_LAST) {
+ if (size == next_item - words.begin()) {
+ if (next_item < words.end() && next_item[2] == PICOBIN_BLOCK_MARKER_END) {
+ DEBUG_LOG("is a valid block\n");
+ new_first_block = block::parse(next_block_addr, next_item + 1, words.begin(), words.begin() + size);
+ break;
+ }
+ }
+ } else {
+ next_item += size;
+ }
+ }
+ if (new_first_block == nullptr) {
+ fail(ERROR_UNKNOWN, "Block loop is not valid - incomplete block found at %08x\n", (int)(next_block_addr));
+ }
+ if (new_first_block->physical_addr + new_first_block->next_block_rel == first_block->physical_addr) {
+ DEBUG_LOG("Found last block in block loop\n");
+ all_blocks.push_back(std::move(new_first_block));
+ break;
+ } else {
+ DEBUG_LOG("Continue looping\n");
+ next_block_addr = new_first_block->physical_addr + new_first_block->next_block_rel;
+ if (next_block_addrs.find(next_block_addr) != next_block_addrs.end()) {
+ fail(ERROR_UNKNOWN,
+ "Block loop is not valid - contains a loop from %08x to %08x, but first block at %08x\n",
+ (int)(new_first_block->physical_addr), (int)(next_block_addr), (int)(first_block->physical_addr)
+ );
+ }
+ next_block_addrs.insert(next_block_addr);
+ all_blocks.push_back(std::move(new_first_block));
+ }
+ }
+ return all_blocks;
+}
+
+
std::vector<std::unique_ptr<block>> get_all_blocks(std::vector<uint8_t> &bin, uint32_t storage_addr, std::unique_ptr<block> &first_block, get_more_bin_cb more_cb) {
uint32_t next_block_addr = first_block->physical_addr + first_block->next_block_rel;
+ std::set<uint32_t> next_block_addrs = {next_block_addr};
std::vector<std::unique_ptr<block>> all_blocks;
uint32_t read_size = PICOBIN_MAX_BLOCK_SIZE;
uint32_t current_bin_start = storage_addr;
@@ -406,6 +433,13 @@
} else {
DEBUG_LOG("Continue looping\n");
next_block_addr = new_first_block->physical_addr + new_first_block->next_block_rel;
+ if (next_block_addrs.find(next_block_addr) != next_block_addrs.end()) {
+ fail(ERROR_UNKNOWN,
+ "Block loop is not valid - contains a loop from %08x to %08x, but first block at %08x\n",
+ (int)(new_first_block->physical_addr), (int)(next_block_addr), (int)(first_block->physical_addr)
+ );
+ }
+ next_block_addrs.insert(next_block_addr);
all_blocks.push_back(std::move(new_first_block));
}
}
@@ -428,6 +462,8 @@
const uint32_t psize = bin.size();
if (paddr >= model->sram_start() && paddr < model->sram_striped_end()) {
highest_ram_address = std::max(paddr + psize, highest_ram_address);
+ } else if (paddr >= model->xip_sram_start() && paddr < model->xip_sram_end()) {
+ highest_ram_address = std::max(paddr + psize, highest_ram_address);
} else if (paddr >= model->flash_start() && paddr < model->flash_end()) {
highest_flash_address = std::max(paddr + psize, highest_flash_address);
}
@@ -454,6 +490,7 @@
if (set_others_ignored) set_block_ignored(bin, storage_addr, first_block->physical_addr);
} else {
DEBUG_LOG("Ooh, there is already a block loop - lets find it's end\n");
+ if (set_others_ignored) set_block_ignored(bin, storage_addr, first_block->physical_addr);
auto all_blocks = get_all_blocks(bin, storage_addr, first_block);
for (auto &block : all_blocks) {
if (set_others_ignored) set_block_ignored(bin, storage_addr, block->physical_addr);
diff --git a/bintool/bintool.h b/bintool/bintool.h
index f8c60ff..d13f150 100644
--- a/bintool/bintool.h
+++ b/bintool/bintool.h
@@ -25,6 +25,7 @@
// Elfs
std::unique_ptr<block> find_first_block(elf_file *elf);
+std::vector<std::unique_ptr<block>> get_all_blocks(elf_file *elf, std::unique_ptr<block> &first_block);
block place_new_block(elf_file *elf, std::unique_ptr<block> &first_block, model_t model, bool set_others_ignored=false);
#if HAS_MBEDTLS
int hash_andor_sign(elf_file *elf, block *new_block, const public_t public_key, const private_t private_key, model_t model, bool hash_value, bool sign, bool clear_sram = false, bool pin_xip_sram = false);
diff --git a/elf/elf_file.cpp b/elf/elf_file.cpp
index d5cd5e9..d4f8d53 100644
--- a/elf/elf_file.cpp
+++ b/elf/elf_file.cpp
@@ -527,6 +527,18 @@
return nullptr;
}
+bool elf_file::physical_address_range_is_occupied(uint32_t paddr, uint32_t psize) {
+ uint32_t pstart = paddr;
+ uint32_t pend = paddr + psize;
+ for (int i = 0; i < eh.ph_num; i++) {
+ if (!ph_entries[i].is_load() || ph_entries[i].filez == 0) continue; // allow overlapping with non-loaded segments
+ if (pstart < ph_entries[i].paddr + ph_entries[i].filez && ph_entries[i].paddr < pend) {
+ return true;
+ }
+ }
+ return false;
+}
+
const elf32_ph_entry* elf_file::segment_from_virtual_address(uint32_t vaddr) {
for (int i = 0; i < eh.ph_num; i++) {
if (vaddr >= ph_entries[i].vaddr && vaddr < ph_entries[i].vaddr + ph_entries[i].memsz) {
@@ -567,49 +579,44 @@
return const_sorted_segs;
}
-void elf_file::store_squashed(model_t model) {
- uint32_t highest_ram_address = 0;
- uint32_t highest_flash_address = 0;
-
+void elf_file::store_squashed(model_t model, const std::vector<uint32_t> &pinned_addresses) {
for(const auto &seg : sorted_segments()) {
const uint32_t paddr = seg->physical_address();
const uint32_t psize = seg->physical_size();
if (psize == 0) continue;
- if (paddr >= model->sram_start() && paddr < model->sram_striped_end()) {
- highest_ram_address = std::max(paddr + psize, highest_ram_address);
- } else if (paddr >= model->flash_start() && paddr < model->flash_end()) {
- highest_flash_address = std::max(paddr + psize, highest_flash_address);
+ if (paddr >= model->flash_start() && paddr < model->flash_end()) {
+ // cannot squash flash binaries
+ return;
}
}
- if (highest_flash_address != 0) {
- // cannot squash flash binaries
- return;
- }
-
uint32_t last_seg_end = 0;
- uint32_t last_seg_start = 0;
std::vector<elf32_ph_entry *> xip_sram_segs = {};
+ auto contains_pinned_address = [&](const elf32_ph_entry *seg) {
+ for (uint32_t addr : pinned_addresses) {
+ if (addr >= seg->physical_address() && addr < seg->physical_address() + seg->physical_size()) {
+ return true;
+ }
+ }
+ return false;
+ };
+
auto squash_seg = [&](elf32_ph_entry *seg) {
const uint32_t paddr = seg->physical_address();
const uint32_t psize = seg->physical_size();
if (!seg->is_load()) return;
-
- const bool is_alias = paddr == last_seg_start;
- last_seg_start = paddr;
-
- if (psize == 0 || is_alias) return;
+ if (psize == 0) return;
if (last_seg_end) {
- if (paddr != last_seg_end) {
+ if (paddr != last_seg_end && !contains_pinned_address(seg) && !physical_address_range_is_occupied(last_seg_end, psize)) {
if (verbose) printf("squashing %08x to %08x\n", paddr, last_seg_end);
seg->paddr = last_seg_end;
}
}
- // May have been modified, so read again
- last_seg_end = seg->physical_address() + seg->physical_size();
+ // May have been modified, so read again. Use max so last_seg_end never moves backwards.
+ last_seg_end = std::max(last_seg_end, seg->physical_address() + seg->physical_size());
};
for(auto &seg : sorted_segments_modifiable()) {
diff --git a/elf/elf_file.h b/elf/elf_file.h
index 7b06a88..96c147d 100644
--- a/elf/elf_file.h
+++ b/elf/elf_file.h
@@ -46,8 +46,9 @@
const elf32_ph_entry* segment_from_physical_address(uint32_t paddr);
const elf32_ph_entry* segment_from_virtual_address(uint32_t vaddr);
const elf32_ph_entry* segment_from_section(const elf32_sh_entry &sh);
+ bool physical_address_range_is_occupied(uint32_t paddr, uint32_t psize);
void dump(void) const;
- void store_squashed(model_t model);
+ void store_squashed(model_t model, const std::vector<uint32_t> &pinned_addresses = {});
void move_all(int dist);
void remove_ph_holes(void);
diff --git a/main.cpp b/main.cpp
index 9d042bb..d53d530 100644
--- a/main.cpp
+++ b/main.cpp
@@ -6184,8 +6184,20 @@
elf_file *elf = &source_file;
elf->read_file(get_file(ios::in|ios::binary));
if (!settings.seal.no_squash) {
+ // Segments containing a block that is already part of a metadata block loop must not be
+ // moved, as other blocks in the loop link to them by absolute physical address
+ std::vector<uint32_t> pinned_addresses;
+ std::unique_ptr<block> first_block = find_first_block(elf);
+ if (first_block) {
+ pinned_addresses.push_back(first_block->physical_addr);
+ if (first_block->next_block_rel) {
+ for (auto &blk : get_all_blocks(elf, first_block)) {
+ pinned_addresses.push_back(blk->physical_addr);
+ }
+ }
+ }
// Squash the segments together
- elf->store_squashed(model);
+ elf->store_squashed(model, pinned_addresses);
}
// Remove any holes in the ELF file, as these cause issues when signing/hashing
elf->remove_sh_holes();
