From f73dc812e1245df61ea57f8972b7a785deea1b8a Mon Sep 17 00:00:00 2001 From: William Vinnicombe Date: Fri, 13 Jun 2025 12:30:32 +0100 Subject: [PATCH 1/3] Fix encryption when using clang, and with allocatable heap --- bintool/bintool.cpp | 4 ++++ elf/elf_file.cpp | 39 ++++++++++++++++++++++++++++++++++++++- elf/elf_file.h | 1 + main.cpp | 1 + 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/bintool/bintool.cpp b/bintool/bintool.cpp index 8d5cb22..26b8b42 100644 --- a/bintool/bintool.cpp +++ b/bintool/bintool.cpp @@ -263,6 +263,7 @@ block place_new_block(elf_file *elf, std::unique_ptr &first_block, bool s // std::cout << &seg << " " << to_string(seg) << std::endl; const uint32_t paddr = seg.physical_address(); const uint32_t psize = seg.physical_size(); + if (psize == 0) continue; if (paddr >= 0x20000000 && paddr < 0x20080000) { highest_ram_address = std::max(paddr + psize, highest_ram_address); } else if (paddr >= 0x10000000 && paddr < 0x20000000) { @@ -406,6 +407,9 @@ std::vector> get_all_blocks(std::vector &bin, ui 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)); diff --git a/elf/elf_file.cpp b/elf/elf_file.cpp index 827e705..5871c5b 100644 --- a/elf/elf_file.cpp +++ b/elf/elf_file.cpp @@ -251,7 +251,33 @@ void elf_file::read_sh(void) { } } -// If there are holes between sections within segments, increase the section size to plug the hole +// If there are holes between segments, increase the segment size to plug the hole +// This is necessary to ensure that segments are contiguous, which is required for encrypting, +// but this is not necessary for signing/hashing +void elf_file::remove_ph_holes(void) { + for (int i=0; i+1 < ph_entries.size(); i++) { + auto ph0 = &(ph_entries[i]); + elf32_ph_entry ph1 = ph_entries[i+1]; + if ( + (ph0->type == PT_LOAD && ph1.type == PT_LOAD) + && (ph0->filez && ph1.filez) + && (ph0->paddr + ph0->filez < ph1.paddr) + ) { + uint32_t gap = ph1.paddr - ph0->paddr - ph0->filez; + if (gap > ph1.align) { + fail(ERROR_INCOMPATIBLE, "Segment %d: Cannot plug gap greater than alignment - gap %d, alignment %d", i, gap, ph1.align); + } + if (ph0->offset + ph0->filez + gap > ph1.offset) { + fail(ERROR_INCOMPATIBLE, "Segment %d: Cannot plug gap without space in file - gap %d", i, gap); + } + if (verbose) printf("Segment %d: Moving end from 0x%08x to 0x%08x to plug gap\n", i, ph0->paddr + ph0->filez, ph1.paddr); + ph0->filez = ph1.paddr - ph0->paddr; + ph0->memsz = ph0->filez; + } + } +} + +// If there are holes within segments, increase the section size to plug the hole // This is necessary to ensure the whole segment contains data defined in sections, otherwise you end up // signing/hashing/encrypting data that may not be written, as many tools write in sections not segments void elf_file::remove_sh_holes(void) { @@ -273,6 +299,17 @@ void elf_file::remove_sh_holes(void) { if (verbose) printf("Section %d: Moving end from 0x%08x to 0x%08x to plug gap\n", i, sh0->addr + sh0->size, sh1.addr); sh0->size = sh1.addr - sh0->addr; found_hole = true; + } else if ( + (sh0->type == SHT_PROGBITS) + && (segment_from_section(*sh0) != segment_from_section(sh1)) + && segment_from_section(*sh0) != NULL + && (sh0->offset + sh0->size < segment_from_section(*sh0)->offset + segment_from_section(*sh0)->filez) + ) { + const elf32_ph_entry *seg = segment_from_section(*sh0); + uint32_t gap = seg->offset + seg->filez - sh0->offset - sh0->size; + if (verbose) printf("Section %d: Moving end from 0x%08x to 0x%08x to plug gap at end of segment\n", i, sh0->addr + sh0->size, seg->offset + seg->filez); + sh0->size = seg->offset + seg->filez - sh0->offset; + found_hole = true; } } if (found_hole) read_sh_data(); diff --git a/elf/elf_file.h b/elf/elf_file.h index 6ca9348..816c50f 100644 --- a/elf/elf_file.h +++ b/elf/elf_file.h @@ -47,6 +47,7 @@ class elf_file { void dump(void) const; void move_all(int dist); + void remove_ph_holes(void); void remove_sh_holes(void); static std::vector read_binfile(std::shared_ptr file); diff --git a/main.cpp b/main.cpp index ee9bdb6..068f88e 100644 --- a/main.cpp +++ b/main.cpp @@ -5171,6 +5171,7 @@ bool encrypt_command::execute(device_map &devices) { elf_file *elf = &source_file; elf->read_file(get_file(ios::in|ios::binary)); // Remove any holes in the ELF file, as these cause issues when encrypting + elf->remove_ph_holes(); elf->remove_sh_holes(); std::unique_ptr first_block = find_first_block(elf); From f54e319f1a2630e23079ebe827b53649d39f37e7 Mon Sep 17 00:00:00 2001 From: William Vinnicombe Date: Fri, 13 Jun 2025 13:45:43 +0100 Subject: [PATCH 2/3] Test build examples with clang too --- .github/workflows/test.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index edd7e4f..bd1521c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -76,12 +76,22 @@ jobs: # Prevent running twice for PRs from same repo if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name name: Test Build Examples + strategy: + fail-fast: false + matrix: + compiler: ["gcc", "clang"] runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@v4 - name: Install dependencies - run: sudo apt install cmake ninja-build python3 build-essential gcc-arm-none-eabi libnewlib-arm-none-eabi libstdc++-arm-none-eabi-newlib libusb-1.0-0-dev + run: sudo apt install cmake ninja-build python3 build-essential libusb-1.0-0-dev + - name: Install GCC + if: matrix.compiler == 'gcc' + run: sudo apt install gcc-arm-none-eabi libnewlib-arm-none-eabi libstdc++-arm-none-eabi-newlib + - name: Install Clang + if: matrix.compiler == 'clang' + uses: stellar-aria/llvm-embedded-toolchain-for-arm-action@latest - name: Checkout Pico SDK uses: actions/checkout@v4 with: @@ -102,5 +112,5 @@ jobs: path: pico-examples - name: Build Pico Examples run: | - cmake -S pico-examples -B build-examples -G "Ninja" -D PICO_SDK_PATH="${{ github.workspace }}/pico-sdk" -D PICO_BOARD=pico2_w + cmake -S pico-examples -B build-examples -G "Ninja" -D PICO_SDK_PATH="${{ github.workspace }}/pico-sdk" -D PICO_BOARD=pico2_w ${{ matrix.compiler == 'clang' && '-D PICO_COMPILER=pico_arm_clang' || '' }} cmake --build build-examples From 3d47b786040d183ec46eabcc46b1c9d4a2c6ffed Mon Sep 17 00:00:00 2001 From: William Vinnicombe Date: Wed, 18 Jun 2025 14:22:25 +0100 Subject: [PATCH 3/3] Delete empty PH segments when moving These segments are not necessary, and will likely be moved to invalid memory addresses --- elf/elf_file.cpp | 11 +++++++++++ elf/elf_file.h | 1 + 2 files changed, 12 insertions(+) diff --git a/elf/elf_file.cpp b/elf/elf_file.cpp index 5871c5b..5248044 100644 --- a/elf/elf_file.cpp +++ b/elf/elf_file.cpp @@ -315,6 +315,15 @@ void elf_file::remove_sh_holes(void) { if (found_hole) read_sh_data(); } +void elf_file::remove_empty_ph_entries(void) { + for (int i = 0; i < ph_entries.size(); i++) { + if (ph_entries[i].filez == 0) { + ph_entries.erase(ph_entries.begin() + i); + eh.ph_num--; i--; + } + } +} + // Read the section data from the internal byte array into discrete sections. // This is used after modifying segments but before inserting new segments void elf_file::read_sh_data(void) { @@ -410,6 +419,8 @@ void elf_file::dump(void) const { void elf_file::move_all(int dist) { if (verbose) printf("Incrementing all paddr by %d\n", dist); + // Remove empty ph entries, as they will likely be moved to invalid addresses + remove_empty_ph_entries(); for (auto &ph: ph_entries) { ph.paddr += dist; } diff --git a/elf/elf_file.h b/elf/elf_file.h index 816c50f..8c2bff8 100644 --- a/elf/elf_file.h +++ b/elf/elf_file.h @@ -49,6 +49,7 @@ class elf_file { void move_all(int dist); void remove_ph_holes(void); void remove_sh_holes(void); + void remove_empty_ph_entries(void); static std::vector read_binfile(std::shared_ptr file);