* [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig @ 2026-06-13 19:56 Johannes Schneider 2026-06-15 9:49 ` Ahmad Fatoum 0 siblings, 1 reply; 5+ messages in thread From: Johannes Schneider @ 2026-06-13 19:56 UTC (permalink / raw) To: barebox; +Cc: Johannes Schneider The PBL verifies fw-external blobs by hashing them in DRAM after the BootROM-supplied load, before handing off to TF-A. SHA-256 is the only choice today, and it runs with the MMU off and DRAM reads uncached -- no D-cache, no hardware crypto, plain software hash walking DRAM through the slowest available access path. The cost is non-trivial. Measured on Cortex-A53, MMU off, uncached DRAM: SHA-256, ~720 KiB blob: ~2 s CRC32, ~720 KiB blob: ~10 ms That ~2 s sits in PBL phase 1, before BL31 entry, and is paid every boot. When the SoC's secure boot (HABv4 on i.MX8M, AHAB on i.MX9, or an upstream-signed FIT / bundle) already authenticates the blob before the PBL ever sees it, the in-PBL SHA-256 is redundant: it only re-checks the BootROM->DRAM copy against a value baked into the PBL itself, not against an external trust anchor. A 32-bit CRC over the same window catches the realistic failure mode (a corrupted copy into DRAM) at ~200x lower wall-clock cost. Make the verify algorithm a Kconfig choice: PBL_FIRMWARE_EXT_VERIFY_NONE no verify PBL_FIRMWARE_EXT_VERIFY_CRC32 4-byte CRC, cheap corruption check PBL_FIRMWARE_EXT_VERIFY_SHA256 current behaviour, default firmware/Makefile selects an algorithm and passes it to gen-fw-s, which emits nothing for NONE, a 4-byte .long for CRC32, or 32 bytes for SHA-256 in the per-blob .rodata.<fwstr>.sha section. The macro in include/firmware.h declares the verify symbols only when actually used (offset != 0, i.e. fw-external blobs), so NONE is a true no-op and in-tree firmware never grew a section dependency. firmware_ext_verify() dispatches via IS_ENABLED() at compile time. The verify-section symbol names (_fw_*_sha_*) are kept as-is rather than renamed, since firmware_next_image_verify() also references them for the FIRMWARE_VERIFY_NEXT_IMAGE path; under CRC32 the name no longer describes the contents, but renaming would expand the patch scope unnecessarily. Default stays at SHA-256 so existing defconfigs keep their current semantics. Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com> --- firmware/Kconfig | 33 +++++++++++++++++++++++++++ firmware/Makefile | 15 ++++++++++-- include/firmware.h | 41 +++++++++++++++++++++++---------- include/pbl.h | 2 ++ pbl/decomp.c | 35 ++++++++++++++++++++++++++++ scripts/gen-fw-s | 57 +++++++++++++++++++++++++++++++++------------- 6 files changed, 153 insertions(+), 30 deletions(-) diff --git a/firmware/Kconfig b/firmware/Kconfig index b9b4556dbd..f745819a67 100644 --- a/firmware/Kconfig +++ b/firmware/Kconfig @@ -151,4 +151,37 @@ config FIRMWARE_VERIFY_NEXT_IMAGE verified images. The function to check the next stage image hash is firmware_next_image_verify(), make sure your SoC code uses it. +choice + prompt "PBL fw-external blob verification" + default PBL_FIRMWARE_EXT_VERIFY_SHA256 + help + Algorithm used by the PBL to verify an fw-external blob against a + value embedded at build time, before handoff to TF-A. Runs with + the MMU off and DRAM reads uncached, so cost scales sharply with + blob size and algorithm. This checks integrity vs the build-time + value; it is not authentication against an external trust anchor. + +config PBL_FIRMWARE_EXT_VERIFY_NONE + bool "none" + help + Skip the verify. Appropriate when the boot chain authenticates the + blob upstream of the PBL (HAB, AHAB, signed FIT, ...) -- the + in-PBL check is then redundant. + +config PBL_FIRMWARE_EXT_VERIFY_CRC32 + bool "CRC32" + select CRC32 + help + 4-byte CRC32 (zlib polynomial). Catches accidental corruption + of the BootROM->DRAM copy; not cryptographic. + +config PBL_FIRMWARE_EXT_VERIFY_SHA256 + bool "SHA-256" + help + Cryptographic hash, software-computed with MMU off. Strongest + option but by far the slowest -- prefer NONE or CRC32 when the + blob is already authenticated upstream of the PBL. + +endchoice + endmenu diff --git a/firmware/Makefile b/firmware/Makefile index 7e433a1824..1590bed9f1 100644 --- a/firmware/Makefile +++ b/firmware/Makefile @@ -61,8 +61,19 @@ pbl-fwext-y := $(addsuffix .extgen.o, $(fw-external-y)) FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)) -filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) -filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) +# Verify algorithm passed to gen-fw-s for fw-external blobs. Drives the +# verify metadata section emitted into .extgen.S and matched by +# firmware_ext_verify() in include/firmware.h. +ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE +fw_verify_algo := none +else ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 +fw_verify_algo := crc32 +else +fw_verify_algo := sha256 +endif + +filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) sha256 +filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) $(fw_verify_algo) $(obj)/%.gen.S: FORCE $(call filechk,fwbin) diff --git a/include/firmware.h b/include/firmware.h index 6511d56b2e..72a47f24e3 100644 --- a/include/firmware.h +++ b/include/firmware.h @@ -80,13 +80,21 @@ static inline void release_firmware(const struct firmware *fw) void firmwaremgr_list_handlers(void); -static inline void firmware_ext_verify(const void *data_start, size_t data_size, - const void *hash_start, size_t hash_size) +static inline void firmware_ext_verify(const void *data, size_t data_size, + const void *verify, size_t verify_size) { - if (pbl_barebox_verify(data_start, data_size, - hash_start, hash_size) != 0) { + int ret; + + if (IS_ENABLED(CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32)) + ret = pbl_barebox_verify_crc32(data, data_size, + verify, verify_size); + else + ret = pbl_barebox_verify(data, data_size, + verify, verify_size); + + if (ret != 0) { putc_ll('!'); - panic("hash mismatch, refusing to decompress"); + panic("firmware verify mismatch, refusing to decompress"); } } @@ -96,22 +104,31 @@ struct fwobj { void *data; }; +#if defined(CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE) +# define __fw_ext_verify(name, fwobj) do { } while (0) +#else +# define __fw_ext_verify(name, fwobj) \ + do { \ + extern char _fw_##name##_sha_start[]; \ + extern char _fw_##name##_sha_end[]; \ + firmware_ext_verify( \ + (fwobj)->data, (fwobj)->size, \ + _fw_##name##_sha_start, \ + _fw_##name##_sha_end - \ + _fw_##name##_sha_start); \ + } while (0) +#endif + #define __get_builtin_firmware(name, offset, fwobj) \ do { \ extern char _fw_##name##_start[]; \ extern char _fw_##name##_end[]; \ - extern char _fw_##name##_sha_start[]; \ - extern char _fw_##name##_sha_end[]; \ (fwobj)->data = _fw_##name##_start; \ (fwobj)->size = _fw_##name##_end - _fw_##name##_start; \ if (!(offset)) \ break; \ (fwobj)->data += (offset); \ - firmware_ext_verify( \ - (fwobj)->data, (fwobj)->size, \ - _fw_##name##_sha_start, \ - _fw_##name##_sha_end - _fw_##name##_sha_start \ - ); \ + __fw_ext_verify(name, fwobj); \ } while (0) diff --git a/include/pbl.h b/include/pbl.h index fe4367825c..57ca8b4eb3 100644 --- a/include/pbl.h +++ b/include/pbl.h @@ -29,6 +29,8 @@ fdt_device_get_match_data(const void *fdt, const char *nodepath, int pbl_barebox_verify(const void *compressed_start, unsigned int len, const void *hash, unsigned int hash_len); +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, + const void *crc, unsigned int crc_len); int pbl_load_fdt(const void *fdt, void *dest, int destsize); #define PBL_MALLOC_SIZE SZ_128K diff --git a/pbl/decomp.c b/pbl/decomp.c index 1539a6b67e..ace121eac6 100644 --- a/pbl/decomp.c +++ b/pbl/decomp.c @@ -13,6 +13,10 @@ #include <asm/sections.h> #include <pbl.h> #include <debug_ll.h> +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 +#include <crc.h> +#include <asm/unaligned.h> +#endif #define STATIC static @@ -90,6 +94,37 @@ int pbl_barebox_verify(const void *compressed_start, unsigned int len, return memcmp(hash, computed_hash, SHA256_DIGEST_SIZE); } +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, + const void *crc, unsigned int crc_len) +{ + uint32_t expected, computed; + + if (crc_len != sizeof(uint32_t)) + return -1; + + /* Stored little-endian by .long in the .rodata.*.sha section. */ + expected = get_unaligned_le32(crc); + computed = crc32(0, data_start, len); + + if (IS_ENABLED(CONFIG_DEBUG_LL)) { + puts_ll("CRC "); + puthexc_ll((computed >> 24) & 0xff); + puthexc_ll((computed >> 16) & 0xff); + puthexc_ll((computed >> 8) & 0xff); + puthexc_ll(computed & 0xff); + puts_ll(" vs "); + puthexc_ll((expected >> 24) & 0xff); + puthexc_ll((expected >> 16) & 0xff); + puthexc_ll((expected >> 8) & 0xff); + puthexc_ll(expected & 0xff); + putc_ll('\n'); + } + + return computed == expected ? 0 : -1; +} +#endif + void pbl_barebox_uncompress(void *dest, void *compressed_start, unsigned int len) { uint32_t pbl_hash_len; diff --git a/scripts/gen-fw-s b/scripts/gen-fw-s index 78c3193479..d7d7d15824 100755 --- a/scripts/gen-fw-s +++ b/scripts/gen-fw-s @@ -3,13 +3,20 @@ # # Generate assembly source to embed firmware binary # -# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] +# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] [verify-algo] +# +# verify-algo selects the metadata emitted in the .rodata.<fwstr>.sha +# section, matched by firmware_ext_verify() in include/firmware.h: +# sha256 (default) -- 32-byte SHA-256 +# crc32 -- 4-byte CRC32 (zlib polynomial) +# none -- no section, no symbols fwname=$1 fwdir=$2 secprefix=$3 secflags=$4 fwobjdir=$5 +verify_algo=${6:-sha256} fwstr=$(echo "$fwname" | tr '/.-' '___') fwpath="$fwdir/$fwname" @@ -19,8 +26,6 @@ if [ -f "$fwpath" ]; then fw_uncompressed=$(stat -c %s "$fwpath") fi -sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') - echo "/* Generated by scripts/gen-fw-s */" echo "#include <asm-generic/pointer.h>" echo ".section .note.GNU-stack,\"\",%progbits" @@ -59,19 +64,39 @@ echo ".global _fw_z_${fwstr}_end" echo "_fw_z_${fwstr}_end:" echo "#endif" -# include sha256, needed for external firmware -echo " .section .rodata.${fwstr}.sha" -echo " .p2align ASM_LGPTR" -echo ".global _fw_${fwstr}_sha_start" -echo "_fw_${fwstr}_sha_start:" -echo " .byte ${sha}" -echo ".global _fw_${fwstr}_sha_end" -echo "_fw_${fwstr}_sha_end:" -if [ -f "$fwpath" ]; then - echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" - echo ".error \"sha256sum invalid\"" - echo ".endif" -fi +# verify metadata for fw-external blobs, consumed by firmware_ext_verify() +# in include/firmware.h. Driven by the PBL_FIRMWARE_EXT_VERIFY Kconfig +# choice (passed in from firmware/Makefile). +case "$verify_algo" in +none) + ;; +crc32) + crc=$(python3 -c 'import sys,zlib; sys.stdout.write("0x%08x" % zlib.crc32(open(sys.argv[1],"rb").read()))' \ + "$fwpath" 2>/dev/null || echo "0x00000000") + echo " .section .rodata.${fwstr}.sha" + echo " .p2align 2" + echo ".global _fw_${fwstr}_sha_start" + echo "_fw_${fwstr}_sha_start:" + echo " .long ${crc}" + echo ".global _fw_${fwstr}_sha_end" + echo "_fw_${fwstr}_sha_end:" + ;; +sha256|*) + sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') + echo " .section .rodata.${fwstr}.sha" + echo " .p2align ASM_LGPTR" + echo ".global _fw_${fwstr}_sha_start" + echo "_fw_${fwstr}_sha_start:" + echo " .byte ${sha}" + echo ".global _fw_${fwstr}_sha_end" + echo "_fw_${fwstr}_sha_end:" + if [ -f "$fwpath" ]; then + echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" + echo ".error \"sha256sum invalid\"" + echo ".endif" + fi + ;; +esac # include a string containing the firmware name. When a non existing # firmware is referenced in the PBL then _fwname_${fwstr} is referenced base-commit: 6c70fb327d486376c1f2e37dfff2212cb9eebb1b -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig 2026-06-13 19:56 [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig Johannes Schneider @ 2026-06-15 9:49 ` Ahmad Fatoum 2026-06-16 5:46 ` SCHNEIDER Johannes 0 siblings, 1 reply; 5+ messages in thread From: Ahmad Fatoum @ 2026-06-15 9:49 UTC (permalink / raw) To: Johannes Schneider, barebox Hello Johannes, On 6/13/26 9:56 PM, Johannes Schneider wrote: > The PBL verifies fw-external blobs by hashing them in DRAM after the > BootROM-supplied load, before handing off to TF-A. SHA-256 is the > only choice today, and it runs with the MMU off and DRAM reads > uncached -- no D-cache, no hardware crypto, plain software hash > walking DRAM through the slowest available access path. The cost is > non-trivial. > > Measured on Cortex-A53, MMU off, uncached DRAM: > > SHA-256, ~720 KiB blob: ~2 s > CRC32, ~720 KiB blob: ~10 ms > > That ~2 s sits in PBL phase 1, before BL31 entry, and is paid every > boot. When the SoC's secure boot (HABv4 on i.MX8M, AHAB on i.MX9, or > an upstream-signed FIT / bundle) already authenticates the blob > before the PBL ever sees it, the in-PBL SHA-256 is redundant: This is incorrect. The BootROM can only load as many bytes as it can fit into on-chip SRAM. The loaded PBL will set up DRAM and then loads the full image into DRAM. Replacing SHA256 here with CRC32 thus breaks verified boot from EL2 onward. > it > only re-checks the BootROM->DRAM copy against a value baked into the > PBL itself, not against an external trust anchor. A 32-bit CRC over > the same window catches the realistic failure mode (a corrupted copy > into DRAM) at ~200x lower wall-clock cost. Give recent commit f2ae1a4a85d0 ("ARM: rockchip: atf: enable MMU in PBL") and its follow-up a look. I'd suggest you do the same: Keep the hash as-is, but set up MMU as soon as you have the memory size. You can also play around with mimicking compression of firmware blobs like we have on Rockchip. Depending on CPU vs I/O speed, this can also reduce your startup time. Cheers, Ahmad > > Make the verify algorithm a Kconfig choice: > > PBL_FIRMWARE_EXT_VERIFY_NONE no verify > PBL_FIRMWARE_EXT_VERIFY_CRC32 4-byte CRC, cheap corruption check > PBL_FIRMWARE_EXT_VERIFY_SHA256 current behaviour, default > > firmware/Makefile selects an algorithm and passes it to gen-fw-s, > which emits nothing for NONE, a 4-byte .long for CRC32, or 32 bytes > for SHA-256 in the per-blob .rodata.<fwstr>.sha section. The macro > in include/firmware.h declares the verify symbols only when actually > used (offset != 0, i.e. fw-external blobs), so NONE is a true no-op > and in-tree firmware never grew a section dependency. > firmware_ext_verify() dispatches via IS_ENABLED() at compile time. > > The verify-section symbol names (_fw_*_sha_*) are kept as-is rather > than renamed, since firmware_next_image_verify() also references > them for the FIRMWARE_VERIFY_NEXT_IMAGE path; under CRC32 the name > no longer describes the contents, but renaming would expand the > patch scope unnecessarily. > > Default stays at SHA-256 so existing defconfigs keep their current > semantics. > > Assisted-by: Claude:claude-opus-4-7 > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com> > --- > firmware/Kconfig | 33 +++++++++++++++++++++++++++ > firmware/Makefile | 15 ++++++++++-- > include/firmware.h | 41 +++++++++++++++++++++++---------- > include/pbl.h | 2 ++ > pbl/decomp.c | 35 ++++++++++++++++++++++++++++ > scripts/gen-fw-s | 57 +++++++++++++++++++++++++++++++++------------- > 6 files changed, 153 insertions(+), 30 deletions(-) > > diff --git a/firmware/Kconfig b/firmware/Kconfig > index b9b4556dbd..f745819a67 100644 > --- a/firmware/Kconfig > +++ b/firmware/Kconfig > @@ -151,4 +151,37 @@ config FIRMWARE_VERIFY_NEXT_IMAGE > verified images. The function to check the next stage image hash is > firmware_next_image_verify(), make sure your SoC code uses it. > > +choice > + prompt "PBL fw-external blob verification" > + default PBL_FIRMWARE_EXT_VERIFY_SHA256 > + help > + Algorithm used by the PBL to verify an fw-external blob against a > + value embedded at build time, before handoff to TF-A. Runs with > + the MMU off and DRAM reads uncached, so cost scales sharply with > + blob size and algorithm. This checks integrity vs the build-time > + value; it is not authentication against an external trust anchor. > + > +config PBL_FIRMWARE_EXT_VERIFY_NONE > + bool "none" > + help > + Skip the verify. Appropriate when the boot chain authenticates the > + blob upstream of the PBL (HAB, AHAB, signed FIT, ...) -- the > + in-PBL check is then redundant. > + > +config PBL_FIRMWARE_EXT_VERIFY_CRC32 > + bool "CRC32" > + select CRC32 > + help > + 4-byte CRC32 (zlib polynomial). Catches accidental corruption > + of the BootROM->DRAM copy; not cryptographic. > + > +config PBL_FIRMWARE_EXT_VERIFY_SHA256 > + bool "SHA-256" > + help > + Cryptographic hash, software-computed with MMU off. Strongest > + option but by far the slowest -- prefer NONE or CRC32 when the > + blob is already authenticated upstream of the PBL. > + > +endchoice > + > endmenu > diff --git a/firmware/Makefile b/firmware/Makefile > index 7e433a1824..1590bed9f1 100644 > --- a/firmware/Makefile > +++ b/firmware/Makefile > @@ -61,8 +61,19 @@ pbl-fwext-y := $(addsuffix .extgen.o, $(fw-external-y)) > > FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)) > > -filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) > -filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) > +# Verify algorithm passed to gen-fw-s for fw-external blobs. Drives the > +# verify metadata section emitted into .extgen.S and matched by > +# firmware_ext_verify() in include/firmware.h. > +ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE > +fw_verify_algo := none > +else ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 > +fw_verify_algo := crc32 > +else > +fw_verify_algo := sha256 > +endif > + > +filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) sha256 > +filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) $(fw_verify_algo) > > $(obj)/%.gen.S: FORCE > $(call filechk,fwbin) > diff --git a/include/firmware.h b/include/firmware.h > index 6511d56b2e..72a47f24e3 100644 > --- a/include/firmware.h > +++ b/include/firmware.h > @@ -80,13 +80,21 @@ static inline void release_firmware(const struct firmware *fw) > > void firmwaremgr_list_handlers(void); > > -static inline void firmware_ext_verify(const void *data_start, size_t data_size, > - const void *hash_start, size_t hash_size) > +static inline void firmware_ext_verify(const void *data, size_t data_size, > + const void *verify, size_t verify_size) > { > - if (pbl_barebox_verify(data_start, data_size, > - hash_start, hash_size) != 0) { > + int ret; > + > + if (IS_ENABLED(CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32)) > + ret = pbl_barebox_verify_crc32(data, data_size, > + verify, verify_size); > + else > + ret = pbl_barebox_verify(data, data_size, > + verify, verify_size); > + > + if (ret != 0) { > putc_ll('!'); > - panic("hash mismatch, refusing to decompress"); > + panic("firmware verify mismatch, refusing to decompress"); > } > } > > @@ -96,22 +104,31 @@ struct fwobj { > void *data; > }; > > +#if defined(CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE) > +# define __fw_ext_verify(name, fwobj) do { } while (0) > +#else > +# define __fw_ext_verify(name, fwobj) \ > + do { \ > + extern char _fw_##name##_sha_start[]; \ > + extern char _fw_##name##_sha_end[]; \ > + firmware_ext_verify( \ > + (fwobj)->data, (fwobj)->size, \ > + _fw_##name##_sha_start, \ > + _fw_##name##_sha_end - \ > + _fw_##name##_sha_start); \ > + } while (0) > +#endif > + > #define __get_builtin_firmware(name, offset, fwobj) \ > do { \ > extern char _fw_##name##_start[]; \ > extern char _fw_##name##_end[]; \ > - extern char _fw_##name##_sha_start[]; \ > - extern char _fw_##name##_sha_end[]; \ > (fwobj)->data = _fw_##name##_start; \ > (fwobj)->size = _fw_##name##_end - _fw_##name##_start; \ > if (!(offset)) \ > break; \ > (fwobj)->data += (offset); \ > - firmware_ext_verify( \ > - (fwobj)->data, (fwobj)->size, \ > - _fw_##name##_sha_start, \ > - _fw_##name##_sha_end - _fw_##name##_sha_start \ > - ); \ > + __fw_ext_verify(name, fwobj); \ > } while (0) > > > diff --git a/include/pbl.h b/include/pbl.h > index fe4367825c..57ca8b4eb3 100644 > --- a/include/pbl.h > +++ b/include/pbl.h > @@ -29,6 +29,8 @@ fdt_device_get_match_data(const void *fdt, const char *nodepath, > > int pbl_barebox_verify(const void *compressed_start, unsigned int len, > const void *hash, unsigned int hash_len); > +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, > + const void *crc, unsigned int crc_len); > int pbl_load_fdt(const void *fdt, void *dest, int destsize); > > #define PBL_MALLOC_SIZE SZ_128K > diff --git a/pbl/decomp.c b/pbl/decomp.c > index 1539a6b67e..ace121eac6 100644 > --- a/pbl/decomp.c > +++ b/pbl/decomp.c > @@ -13,6 +13,10 @@ > #include <asm/sections.h> > #include <pbl.h> > #include <debug_ll.h> > +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 > +#include <crc.h> > +#include <asm/unaligned.h> > +#endif > > #define STATIC static > > @@ -90,6 +94,37 @@ int pbl_barebox_verify(const void *compressed_start, unsigned int len, > return memcmp(hash, computed_hash, SHA256_DIGEST_SIZE); > } > > +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 > +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, > + const void *crc, unsigned int crc_len) > +{ > + uint32_t expected, computed; > + > + if (crc_len != sizeof(uint32_t)) > + return -1; > + > + /* Stored little-endian by .long in the .rodata.*.sha section. */ > + expected = get_unaligned_le32(crc); > + computed = crc32(0, data_start, len); > + > + if (IS_ENABLED(CONFIG_DEBUG_LL)) { > + puts_ll("CRC "); > + puthexc_ll((computed >> 24) & 0xff); > + puthexc_ll((computed >> 16) & 0xff); > + puthexc_ll((computed >> 8) & 0xff); > + puthexc_ll(computed & 0xff); > + puts_ll(" vs "); > + puthexc_ll((expected >> 24) & 0xff); > + puthexc_ll((expected >> 16) & 0xff); > + puthexc_ll((expected >> 8) & 0xff); > + puthexc_ll(expected & 0xff); > + putc_ll('\n'); > + } > + > + return computed == expected ? 0 : -1; > +} > +#endif > + > void pbl_barebox_uncompress(void *dest, void *compressed_start, unsigned int len) > { > uint32_t pbl_hash_len; > diff --git a/scripts/gen-fw-s b/scripts/gen-fw-s > index 78c3193479..d7d7d15824 100755 > --- a/scripts/gen-fw-s > +++ b/scripts/gen-fw-s > @@ -3,13 +3,20 @@ > # > # Generate assembly source to embed firmware binary > # > -# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] > +# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] [verify-algo] > +# > +# verify-algo selects the metadata emitted in the .rodata.<fwstr>.sha > +# section, matched by firmware_ext_verify() in include/firmware.h: > +# sha256 (default) -- 32-byte SHA-256 > +# crc32 -- 4-byte CRC32 (zlib polynomial) > +# none -- no section, no symbols > > fwname=$1 > fwdir=$2 > secprefix=$3 > secflags=$4 > fwobjdir=$5 > +verify_algo=${6:-sha256} > > fwstr=$(echo "$fwname" | tr '/.-' '___') > fwpath="$fwdir/$fwname" > @@ -19,8 +26,6 @@ if [ -f "$fwpath" ]; then > fw_uncompressed=$(stat -c %s "$fwpath") > fi > > -sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') > - > echo "/* Generated by scripts/gen-fw-s */" > echo "#include <asm-generic/pointer.h>" > echo ".section .note.GNU-stack,\"\",%progbits" > @@ -59,19 +64,39 @@ echo ".global _fw_z_${fwstr}_end" > echo "_fw_z_${fwstr}_end:" > echo "#endif" > > -# include sha256, needed for external firmware > -echo " .section .rodata.${fwstr}.sha" > -echo " .p2align ASM_LGPTR" > -echo ".global _fw_${fwstr}_sha_start" > -echo "_fw_${fwstr}_sha_start:" > -echo " .byte ${sha}" > -echo ".global _fw_${fwstr}_sha_end" > -echo "_fw_${fwstr}_sha_end:" > -if [ -f "$fwpath" ]; then > - echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" > - echo ".error \"sha256sum invalid\"" > - echo ".endif" > -fi > +# verify metadata for fw-external blobs, consumed by firmware_ext_verify() > +# in include/firmware.h. Driven by the PBL_FIRMWARE_EXT_VERIFY Kconfig > +# choice (passed in from firmware/Makefile). > +case "$verify_algo" in > +none) > + ;; > +crc32) > + crc=$(python3 -c 'import sys,zlib; sys.stdout.write("0x%08x" % zlib.crc32(open(sys.argv[1],"rb").read()))' \ > + "$fwpath" 2>/dev/null || echo "0x00000000") > + echo " .section .rodata.${fwstr}.sha" > + echo " .p2align 2" > + echo ".global _fw_${fwstr}_sha_start" > + echo "_fw_${fwstr}_sha_start:" > + echo " .long ${crc}" > + echo ".global _fw_${fwstr}_sha_end" > + echo "_fw_${fwstr}_sha_end:" > + ;; > +sha256|*) > + sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') > + echo " .section .rodata.${fwstr}.sha" > + echo " .p2align ASM_LGPTR" > + echo ".global _fw_${fwstr}_sha_start" > + echo "_fw_${fwstr}_sha_start:" > + echo " .byte ${sha}" > + echo ".global _fw_${fwstr}_sha_end" > + echo "_fw_${fwstr}_sha_end:" > + if [ -f "$fwpath" ]; then > + echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" > + echo ".error \"sha256sum invalid\"" > + echo ".endif" > + fi > + ;; > +esac > > # include a string containing the firmware name. When a non existing > # firmware is referenced in the PBL then _fwname_${fwstr} is referenced > > base-commit: 6c70fb327d486376c1f2e37dfff2212cb9eebb1b -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig 2026-06-15 9:49 ` Ahmad Fatoum @ 2026-06-16 5:46 ` SCHNEIDER Johannes 2026-06-16 8:14 ` Ahmad Fatoum 0 siblings, 1 reply; 5+ messages in thread From: SCHNEIDER Johannes @ 2026-06-16 5:46 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox Hoi Ahmad, > > On 6/13/26 9:56 PM, Johannes Schneider wrote: > > The PBL verifies fw-external blobs by hashing them in DRAM after the > > BootROM-supplied load, before handing off to TF-A. SHA-256 is the > > only choice today, and it runs with the MMU off and DRAM reads > > uncached -- no D-cache, no hardware crypto, plain software hash > > walking DRAM through the slowest available access path. The cost is > > non-trivial. > > > > Measured on Cortex-A53, MMU off, uncached DRAM: > > > > SHA-256, ~720 KiB blob: ~2 s > > CRC32, ~720 KiB blob: ~10 ms > > > > That ~2 s sits in PBL phase 1, before BL31 entry, and is paid every > > boot. When the SoC's secure boot (HABv4 on i.MX8M, AHAB on i.MX9, or > > an upstream-signed FIT / bundle) already authenticates the blob > > before the PBL ever sees it, the in-PBL SHA-256 is redundant: > > This is incorrect. The BootROM can only load as many bytes as it can fit > into on-chip SRAM. The loaded PBL will set up DRAM and then loads the > full image into DRAM. > > Replacing SHA256 here with CRC32 thus breaks verified boot from EL2 onward. > ouch, thanks for pushing back and in the right direction here > > > it > > only re-checks the BootROM->DRAM copy against a value baked into the > > PBL itself, not against an external trust anchor. A 32-bit CRC over > > the same window catches the realistic failure mode (a corrupted copy > > into DRAM) at ~200x lower wall-clock cost. > > Give recent commit f2ae1a4a85d0 ("ARM: rockchip: atf: enable MMU in > PBL") and its follow-up a look. > thanks for the hint, i'm "toying" with the code now, and am wondering: instead of enabling the MMU to speed up the software sha256, would you have any concerns if i would enable and use the hardware sha256 that lives inside the imx8? from my first couple of measurements i get: | Variant | bl32-verify | Delta vs prev | Cumulative drop | | Pre-MMU-on (generic C, uncached) | 2190 ms | -- | -- | | Post-MMU-on (generic C, cached) | 451 ms | -1739 ms | -1739 ms | | Crypto-ext, per 64bytes block | 192 ms | -259 ms | -1998 ms | | Crypto-ext, batched | **5 ms** | -187 ms | -2185 ms | i could send either the "enable mmu" or the "enable hardware hashing" a better follow up to this patch - opinions/preferences? > I'd suggest you do the same: Keep the hash as-is, but set up MMU as soon > as you have the memory size. > > You can also play around with mimicking compression of firmware blobs > like we have on Rockchip. Depending on CPU vs I/O speed, this can also > reduce your startup time. compressing the optee tee.bin (~720KiB) and having PBL uncompress it again would (probably) work, i have some code that does this - but for some reason the way we build and sign the whole barebox binary now doesn't agree with the imx8m* bootrom, and it drops me back into serial-dl mode = it refuses the image completly. Any hints for what i need to look out for when adding such feature to the PBL? gruß Johannes > > Cheers, > Ahmad > > > > > Make the verify algorithm a Kconfig choice: > > > > PBL_FIRMWARE_EXT_VERIFY_NONE no verify > > PBL_FIRMWARE_EXT_VERIFY_CRC32 4-byte CRC, cheap corruption check > > PBL_FIRMWARE_EXT_VERIFY_SHA256 current behaviour, default > > > > firmware/Makefile selects an algorithm and passes it to gen-fw-s, > > which emits nothing for NONE, a 4-byte .long for CRC32, or 32 bytes > > for SHA-256 in the per-blob .rodata.<fwstr>.sha section. The macro > > in include/firmware.h declares the verify symbols only when actually > > used (offset != 0, i.e. fw-external blobs), so NONE is a true no-op > > and in-tree firmware never grew a section dependency. > > firmware_ext_verify() dispatches via IS_ENABLED() at compile time. > > > > The verify-section symbol names (_fw_*_sha_*) are kept as-is rather > > than renamed, since firmware_next_image_verify() also references > > them for the FIRMWARE_VERIFY_NEXT_IMAGE path; under CRC32 the name > > no longer describes the contents, but renaming would expand the > > patch scope unnecessarily. > > > > Default stays at SHA-256 so existing defconfigs keep their current > > semantics. > > > > Assisted-by: Claude:claude-opus-4-7 > > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com> > > --- > > firmware/Kconfig | 33 +++++++++++++++++++++++++++ > > firmware/Makefile | 15 ++++++++++-- > > include/firmware.h | 41 +++++++++++++++++++++++---------- > > include/pbl.h | 2 ++ > > pbl/decomp.c | 35 ++++++++++++++++++++++++++++ > > scripts/gen-fw-s | 57 +++++++++++++++++++++++++++++++++------------- > > 6 files changed, 153 insertions(+), 30 deletions(-) > > > > diff --git a/firmware/Kconfig b/firmware/Kconfig > > index b9b4556dbd..f745819a67 100644 > > --- a/firmware/Kconfig > > +++ b/firmware/Kconfig > > @@ -151,4 +151,37 @@ config FIRMWARE_VERIFY_NEXT_IMAGE > > verified images. The function to check the next stage image hash is > > firmware_next_image_verify(), make sure your SoC code uses it. > > > > +choice > > + prompt "PBL fw-external blob verification" > > + default PBL_FIRMWARE_EXT_VERIFY_SHA256 > > + help > > + Algorithm used by the PBL to verify an fw-external blob against a > > + value embedded at build time, before handoff to TF-A. Runs with > > + the MMU off and DRAM reads uncached, so cost scales sharply with > > + blob size and algorithm. This checks integrity vs the build-time > > + value; it is not authentication against an external trust anchor. > > + > > +config PBL_FIRMWARE_EXT_VERIFY_NONE > > + bool "none" > > + help > > + Skip the verify. Appropriate when the boot chain authenticates the > > + blob upstream of the PBL (HAB, AHAB, signed FIT, ...) -- the > > + in-PBL check is then redundant. > > + > > +config PBL_FIRMWARE_EXT_VERIFY_CRC32 > > + bool "CRC32" > > + select CRC32 > > + help > > + 4-byte CRC32 (zlib polynomial). Catches accidental corruption > > + of the BootROM->DRAM copy; not cryptographic. > > + > > +config PBL_FIRMWARE_EXT_VERIFY_SHA256 > > + bool "SHA-256" > > + help > > + Cryptographic hash, software-computed with MMU off. Strongest > > + option but by far the slowest -- prefer NONE or CRC32 when the > > + blob is already authenticated upstream of the PBL. > > + > > +endchoice > > + > > endmenu > > diff --git a/firmware/Makefile b/firmware/Makefile > > index 7e433a1824..1590bed9f1 100644 > > --- a/firmware/Makefile > > +++ b/firmware/Makefile > > @@ -61,8 +61,19 @@ pbl-fwext-y := $(addsuffix .extgen.o, $(fw-external-y)) > > > > FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)) > > > > -filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) > > -filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) > > +# Verify algorithm passed to gen-fw-s for fw-external blobs. Drives the > > +# verify metadata section emitted into .extgen.S and matched by > > +# firmware_ext_verify() in include/firmware.h. > > +ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE > > +fw_verify_algo := none > > +else ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 > > +fw_verify_algo := crc32 > > +else > > +fw_verify_algo := sha256 > > +endif > > + > > +filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) sha256 > > +filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) $(fw_verify_algo) > > > > $(obj)/%.gen.S: FORCE > > $(call filechk,fwbin) > > diff --git a/include/firmware.h b/include/firmware.h > > index 6511d56b2e..72a47f24e3 100644 > > --- a/include/firmware.h > > +++ b/include/firmware.h > > @@ -80,13 +80,21 @@ static inline void release_firmware(const struct firmware *fw) > > > > void firmwaremgr_list_handlers(void); > > > > -static inline void firmware_ext_verify(const void *data_start, size_t data_size, > > - const void *hash_start, size_t hash_size) > > +static inline void firmware_ext_verify(const void *data, size_t data_size, > > + const void *verify, size_t verify_size) > > { > > - if (pbl_barebox_verify(data_start, data_size, > > - hash_start, hash_size) != 0) { > > + int ret; > > + > > + if (IS_ENABLED(CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32)) > > + ret = pbl_barebox_verify_crc32(data, data_size, > > + verify, verify_size); > > + else > > + ret = pbl_barebox_verify(data, data_size, > > + verify, verify_size); > > + > > + if (ret != 0) { > > putc_ll('!'); > > - panic("hash mismatch, refusing to decompress"); > > + panic("firmware verify mismatch, refusing to decompress"); > > } > > } > > > > @@ -96,22 +104,31 @@ struct fwobj { > > void *data; > > }; > > > > +#if defined(CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE) > > +# define __fw_ext_verify(name, fwobj) do { } while (0) > > +#else > > +# define __fw_ext_verify(name, fwobj) \ > > + do { \ > > + extern char _fw_##name##_sha_start[]; \ > > + extern char _fw_##name##_sha_end[]; \ > > + firmware_ext_verify( \ > > + (fwobj)->data, (fwobj)->size, \ > > + _fw_##name##_sha_start, \ > > + _fw_##name##_sha_end - \ > > + _fw_##name##_sha_start); \ > > + } while (0) > > +#endif > > + > > #define __get_builtin_firmware(name, offset, fwobj) \ > > do { \ > > extern char _fw_##name##_start[]; \ > > extern char _fw_##name##_end[]; \ > > - extern char _fw_##name##_sha_start[]; \ > > - extern char _fw_##name##_sha_end[]; \ > > (fwobj)->data = _fw_##name##_start; \ > > (fwobj)->size = _fw_##name##_end - _fw_##name##_start; \ > > if (!(offset)) \ > > break; \ > > (fwobj)->data += (offset); \ > > - firmware_ext_verify( \ > > - (fwobj)->data, (fwobj)->size, \ > > - _fw_##name##_sha_start, \ > > - _fw_##name##_sha_end - _fw_##name##_sha_start \ > > - ); \ > > + __fw_ext_verify(name, fwobj); \ > > } while (0) > > > > > > diff --git a/include/pbl.h b/include/pbl.h > > index fe4367825c..57ca8b4eb3 100644 > > --- a/include/pbl.h > > +++ b/include/pbl.h > > @@ -29,6 +29,8 @@ fdt_device_get_match_data(const void *fdt, const char *nodepath, > > > > int pbl_barebox_verify(const void *compressed_start, unsigned int len, > > const void *hash, unsigned int hash_len); > > +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, > > + const void *crc, unsigned int crc_len); > > int pbl_load_fdt(const void *fdt, void *dest, int destsize); > > > > #define PBL_MALLOC_SIZE SZ_128K > > diff --git a/pbl/decomp.c b/pbl/decomp.c > > index 1539a6b67e..ace121eac6 100644 > > --- a/pbl/decomp.c > > +++ b/pbl/decomp.c > > @@ -13,6 +13,10 @@ > > #include <asm/sections.h> > > #include <pbl.h> > > #include <debug_ll.h> > > +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 > > +#include <crc.h> > > +#include <asm/unaligned.h> > > +#endif > > > > #define STATIC static > > > > @@ -90,6 +94,37 @@ int pbl_barebox_verify(const void *compressed_start, unsigned int len, > > return memcmp(hash, computed_hash, SHA256_DIGEST_SIZE); > > } > > > > +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 > > +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, > > + const void *crc, unsigned int crc_len) > > +{ > > + uint32_t expected, computed; > > + > > + if (crc_len != sizeof(uint32_t)) > > + return -1; > > + > > + /* Stored little-endian by .long in the .rodata.*.sha section. */ > > + expected = get_unaligned_le32(crc); > > + computed = crc32(0, data_start, len); > > + > > + if (IS_ENABLED(CONFIG_DEBUG_LL)) { > > + puts_ll("CRC "); > > + puthexc_ll((computed >> 24) & 0xff); > > + puthexc_ll((computed >> 16) & 0xff); > > + puthexc_ll((computed >> 8) & 0xff); > > + puthexc_ll(computed & 0xff); > > + puts_ll(" vs "); > > + puthexc_ll((expected >> 24) & 0xff); > > + puthexc_ll((expected >> 16) & 0xff); > > + puthexc_ll((expected >> 8) & 0xff); > > + puthexc_ll(expected & 0xff); > > + putc_ll('\n'); > > + } > > + > > + return computed == expected ? 0 : -1; > > +} > > +#endif > > + > > void pbl_barebox_uncompress(void *dest, void *compressed_start, unsigned int len) > > { > > uint32_t pbl_hash_len; > > diff --git a/scripts/gen-fw-s b/scripts/gen-fw-s > > index 78c3193479..d7d7d15824 100755 > > --- a/scripts/gen-fw-s > > +++ b/scripts/gen-fw-s > > @@ -3,13 +3,20 @@ > > # > > # Generate assembly source to embed firmware binary > > # > > -# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] > > +# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] [verify-algo] > > +# > > +# verify-algo selects the metadata emitted in the .rodata.<fwstr>.sha > > +# section, matched by firmware_ext_verify() in include/firmware.h: > > +# sha256 (default) -- 32-byte SHA-256 > > +# crc32 -- 4-byte CRC32 (zlib polynomial) > > +# none -- no section, no symbols > > > > fwname=$1 > > fwdir=$2 > > secprefix=$3 > > secflags=$4 > > fwobjdir=$5 > > +verify_algo=${6:-sha256} > > > > fwstr=$(echo "$fwname" | tr '/.-' '___') > > fwpath="$fwdir/$fwname" > > @@ -19,8 +26,6 @@ if [ -f "$fwpath" ]; then > > fw_uncompressed=$(stat -c %s "$fwpath") > > fi > > > > -sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') > > - > > echo "/* Generated by scripts/gen-fw-s */" > > echo "#include <asm-generic/pointer.h>" > > echo ".section .note.GNU-stack,\"\",%progbits" > > @@ -59,19 +64,39 @@ echo ".global _fw_z_${fwstr}_end" > > echo "_fw_z_${fwstr}_end:" > > echo "#endif" > > > > -# include sha256, needed for external firmware > > -echo " .section .rodata.${fwstr}.sha" > > -echo " .p2align ASM_LGPTR" > > -echo ".global _fw_${fwstr}_sha_start" > > -echo "_fw_${fwstr}_sha_start:" > > -echo " .byte ${sha}" > > -echo ".global _fw_${fwstr}_sha_end" > > -echo "_fw_${fwstr}_sha_end:" > > -if [ -f "$fwpath" ]; then > > - echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" > > - echo ".error \"sha256sum invalid\"" > > - echo ".endif" > > -fi > > +# verify metadata for fw-external blobs, consumed by firmware_ext_verify() > > +# in include/firmware.h. Driven by the PBL_FIRMWARE_EXT_VERIFY Kconfig > > +# choice (passed in from firmware/Makefile). > > +case "$verify_algo" in > > +none) > > + ;; > > +crc32) > > + crc=$(python3 -c 'import sys,zlib; sys.stdout.write("0x%08x" % zlib.crc32(open(sys.argv[1],"rb").read()))' \ > > + "$fwpath" 2>/dev/null || echo "0x00000000") > > + echo " .section .rodata.${fwstr}.sha" > > + echo " .p2align 2" > > + echo ".global _fw_${fwstr}_sha_start" > > + echo "_fw_${fwstr}_sha_start:" > > + echo " .long ${crc}" > > + echo ".global _fw_${fwstr}_sha_end" > > + echo "_fw_${fwstr}_sha_end:" > > + ;; > > +sha256|*) > > + sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') > > + echo " .section .rodata.${fwstr}.sha" > > + echo " .p2align ASM_LGPTR" > > + echo ".global _fw_${fwstr}_sha_start" > > + echo "_fw_${fwstr}_sha_start:" > > + echo " .byte ${sha}" > > + echo ".global _fw_${fwstr}_sha_end" > > + echo "_fw_${fwstr}_sha_end:" > > + if [ -f "$fwpath" ]; then > > + echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" > > + echo ".error \"sha256sum invalid\"" > > + echo ".endif" > > + fi > > + ;; > > +esac > > > > # include a string containing the firmware name. When a non existing > > # firmware is referenced in the PBL then _fwname_${fwstr} is referenced > > > > base-commit: 6c70fb327d486376c1f2e37dfff2212cb9eebb1b > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig 2026-06-16 5:46 ` SCHNEIDER Johannes @ 2026-06-16 8:14 ` Ahmad Fatoum 2026-06-16 14:59 ` SCHNEIDER Johannes 0 siblings, 1 reply; 5+ messages in thread From: Ahmad Fatoum @ 2026-06-16 8:14 UTC (permalink / raw) To: SCHNEIDER Johannes; +Cc: barebox Hello Johannes, On 6/16/26 7:46 AM, SCHNEIDER Johannes wrote: >> On 6/13/26 9:56 PM, Johannes Schneider wrote: >> >>> it >>> only re-checks the BootROM->DRAM copy against a value baked into the >>> PBL itself, not against an external trust anchor. A 32-bit CRC over >>> the same window catches the realistic failure mode (a corrupted copy >>> into DRAM) at ~200x lower wall-clock cost. >> >> Give recent commit f2ae1a4a85d0 ("ARM: rockchip: atf: enable MMU in >> PBL") and its follow-up a look. >> > > thanks for the hint, i'm "toying" with the code now, and am wondering: instead > of enabling the MMU to speed up the software sha256, would you have any concerns > if i would enable and use the hardware sha256 that lives inside the imx8? > > from my first couple of measurements i get: > | Variant | bl32-verify | Delta vs prev | Cumulative drop | > | Pre-MMU-on (generic C, uncached) | 2190 ms | -- | -- | > | Post-MMU-on (generic C, cached) | 451 ms | -1739 ms | -1739 ms | > | Crypto-ext, per 64bytes block | 192 ms | -259 ms | -1998 ms | > | Crypto-ext, batched | **5 ms** | -187 ms | -2185 ms | > > i could send either the "enable mmu" or the "enable hardware hashing" a better follow up to this patch - opinions/preferences? Por qué no los dos? :-) >> I'd suggest you do the same: Keep the hash as-is, but set up MMU as soon >> as you have the memory size. >> >> You can also play around with mimicking compression of firmware blobs >> like we have on Rockchip. Depending on CPU vs I/O speed, this can also >> reduce your startup time. > > compressing the optee tee.bin (~720KiB) and having PBL uncompress it again would > (probably) work, i have some code that does this - but for some reason the way > we build and sign the whole barebox binary now doesn't agree with the imx8m* > bootrom, and it drops me back into serial-dl mode = it refuses the image > completly. That's strange. You can continue booting via serial download mode and run the bootrom command. Maybe there's some useful info there on why the boot failed. > Any hints for what i need to look out for when adding such feature to the PBL? I haven't implemented it myself yet, but saw Sascha's patches for Rockchip. I'd suggest you to follow that. Cheers, Ahmad > > > gruß > Johannes > > >> >> Cheers, >> Ahmad >> >>> >>> Make the verify algorithm a Kconfig choice: >>> >>> PBL_FIRMWARE_EXT_VERIFY_NONE no verify >>> PBL_FIRMWARE_EXT_VERIFY_CRC32 4-byte CRC, cheap corruption check >>> PBL_FIRMWARE_EXT_VERIFY_SHA256 current behaviour, default >>> >>> firmware/Makefile selects an algorithm and passes it to gen-fw-s, >>> which emits nothing for NONE, a 4-byte .long for CRC32, or 32 bytes >>> for SHA-256 in the per-blob .rodata.<fwstr>.sha section. The macro >>> in include/firmware.h declares the verify symbols only when actually >>> used (offset != 0, i.e. fw-external blobs), so NONE is a true no-op >>> and in-tree firmware never grew a section dependency. >>> firmware_ext_verify() dispatches via IS_ENABLED() at compile time. >>> >>> The verify-section symbol names (_fw_*_sha_*) are kept as-is rather >>> than renamed, since firmware_next_image_verify() also references >>> them for the FIRMWARE_VERIFY_NEXT_IMAGE path; under CRC32 the name >>> no longer describes the contents, but renaming would expand the >>> patch scope unnecessarily. >>> >>> Default stays at SHA-256 so existing defconfigs keep their current >>> semantics. >>> >>> Assisted-by: Claude:claude-opus-4-7 >>> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com> >>> --- >>> firmware/Kconfig | 33 +++++++++++++++++++++++++++ >>> firmware/Makefile | 15 ++++++++++-- >>> include/firmware.h | 41 +++++++++++++++++++++++---------- >>> include/pbl.h | 2 ++ >>> pbl/decomp.c | 35 ++++++++++++++++++++++++++++ >>> scripts/gen-fw-s | 57 +++++++++++++++++++++++++++++++++------------- >>> 6 files changed, 153 insertions(+), 30 deletions(-) >>> >>> diff --git a/firmware/Kconfig b/firmware/Kconfig >>> index b9b4556dbd..f745819a67 100644 >>> --- a/firmware/Kconfig >>> +++ b/firmware/Kconfig >>> @@ -151,4 +151,37 @@ config FIRMWARE_VERIFY_NEXT_IMAGE >>> verified images. The function to check the next stage image hash is >>> firmware_next_image_verify(), make sure your SoC code uses it. >>> >>> +choice >>> + prompt "PBL fw-external blob verification" >>> + default PBL_FIRMWARE_EXT_VERIFY_SHA256 >>> + help >>> + Algorithm used by the PBL to verify an fw-external blob against a >>> + value embedded at build time, before handoff to TF-A. Runs with >>> + the MMU off and DRAM reads uncached, so cost scales sharply with >>> + blob size and algorithm. This checks integrity vs the build-time >>> + value; it is not authentication against an external trust anchor. >>> + >>> +config PBL_FIRMWARE_EXT_VERIFY_NONE >>> + bool "none" >>> + help >>> + Skip the verify. Appropriate when the boot chain authenticates the >>> + blob upstream of the PBL (HAB, AHAB, signed FIT, ...) -- the >>> + in-PBL check is then redundant. >>> + >>> +config PBL_FIRMWARE_EXT_VERIFY_CRC32 >>> + bool "CRC32" >>> + select CRC32 >>> + help >>> + 4-byte CRC32 (zlib polynomial). Catches accidental corruption >>> + of the BootROM->DRAM copy; not cryptographic. >>> + >>> +config PBL_FIRMWARE_EXT_VERIFY_SHA256 >>> + bool "SHA-256" >>> + help >>> + Cryptographic hash, software-computed with MMU off. Strongest >>> + option but by far the slowest -- prefer NONE or CRC32 when the >>> + blob is already authenticated upstream of the PBL. >>> + >>> +endchoice >>> + >>> endmenu >>> diff --git a/firmware/Makefile b/firmware/Makefile >>> index 7e433a1824..1590bed9f1 100644 >>> --- a/firmware/Makefile >>> +++ b/firmware/Makefile >>> @@ -61,8 +61,19 @@ pbl-fwext-y := $(addsuffix .extgen.o, $(fw-external-y)) >>> >>> FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)) >>> >>> -filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) >>> -filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) >>> +# Verify algorithm passed to gen-fw-s for fw-external blobs. Drives the >>> +# verify metadata section emitted into .extgen.S and matched by >>> +# firmware_ext_verify() in include/firmware.h. >>> +ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE >>> +fw_verify_algo := none >>> +else ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 >>> +fw_verify_algo := crc32 >>> +else >>> +fw_verify_algo := sha256 >>> +endif >>> + >>> +filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) sha256 >>> +filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) $(fw_verify_algo) >>> >>> $(obj)/%.gen.S: FORCE >>> $(call filechk,fwbin) >>> diff --git a/include/firmware.h b/include/firmware.h >>> index 6511d56b2e..72a47f24e3 100644 >>> --- a/include/firmware.h >>> +++ b/include/firmware.h >>> @@ -80,13 +80,21 @@ static inline void release_firmware(const struct firmware *fw) >>> >>> void firmwaremgr_list_handlers(void); >>> >>> -static inline void firmware_ext_verify(const void *data_start, size_t data_size, >>> - const void *hash_start, size_t hash_size) >>> +static inline void firmware_ext_verify(const void *data, size_t data_size, >>> + const void *verify, size_t verify_size) >>> { >>> - if (pbl_barebox_verify(data_start, data_size, >>> - hash_start, hash_size) != 0) { >>> + int ret; >>> + >>> + if (IS_ENABLED(CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32)) >>> + ret = pbl_barebox_verify_crc32(data, data_size, >>> + verify, verify_size); >>> + else >>> + ret = pbl_barebox_verify(data, data_size, >>> + verify, verify_size); >>> + >>> + if (ret != 0) { >>> putc_ll('!'); >>> - panic("hash mismatch, refusing to decompress"); >>> + panic("firmware verify mismatch, refusing to decompress"); >>> } >>> } >>> >>> @@ -96,22 +104,31 @@ struct fwobj { >>> void *data; >>> }; >>> >>> +#if defined(CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE) >>> +# define __fw_ext_verify(name, fwobj) do { } while (0) >>> +#else >>> +# define __fw_ext_verify(name, fwobj) \ >>> + do { \ >>> + extern char _fw_##name##_sha_start[]; \ >>> + extern char _fw_##name##_sha_end[]; \ >>> + firmware_ext_verify( \ >>> + (fwobj)->data, (fwobj)->size, \ >>> + _fw_##name##_sha_start, \ >>> + _fw_##name##_sha_end - \ >>> + _fw_##name##_sha_start); \ >>> + } while (0) >>> +#endif >>> + >>> #define __get_builtin_firmware(name, offset, fwobj) \ >>> do { \ >>> extern char _fw_##name##_start[]; \ >>> extern char _fw_##name##_end[]; \ >>> - extern char _fw_##name##_sha_start[]; \ >>> - extern char _fw_##name##_sha_end[]; \ >>> (fwobj)->data = _fw_##name##_start; \ >>> (fwobj)->size = _fw_##name##_end - _fw_##name##_start; \ >>> if (!(offset)) \ >>> break; \ >>> (fwobj)->data += (offset); \ >>> - firmware_ext_verify( \ >>> - (fwobj)->data, (fwobj)->size, \ >>> - _fw_##name##_sha_start, \ >>> - _fw_##name##_sha_end - _fw_##name##_sha_start \ >>> - ); \ >>> + __fw_ext_verify(name, fwobj); \ >>> } while (0) >>> >>> >>> diff --git a/include/pbl.h b/include/pbl.h >>> index fe4367825c..57ca8b4eb3 100644 >>> --- a/include/pbl.h >>> +++ b/include/pbl.h >>> @@ -29,6 +29,8 @@ fdt_device_get_match_data(const void *fdt, const char *nodepath, >>> >>> int pbl_barebox_verify(const void *compressed_start, unsigned int len, >>> const void *hash, unsigned int hash_len); >>> +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, >>> + const void *crc, unsigned int crc_len); >>> int pbl_load_fdt(const void *fdt, void *dest, int destsize); >>> >>> #define PBL_MALLOC_SIZE SZ_128K >>> diff --git a/pbl/decomp.c b/pbl/decomp.c >>> index 1539a6b67e..ace121eac6 100644 >>> --- a/pbl/decomp.c >>> +++ b/pbl/decomp.c >>> @@ -13,6 +13,10 @@ >>> #include <asm/sections.h> >>> #include <pbl.h> >>> #include <debug_ll.h> >>> +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 >>> +#include <crc.h> >>> +#include <asm/unaligned.h> >>> +#endif >>> >>> #define STATIC static >>> >>> @@ -90,6 +94,37 @@ int pbl_barebox_verify(const void *compressed_start, unsigned int len, >>> return memcmp(hash, computed_hash, SHA256_DIGEST_SIZE); >>> } >>> >>> +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 >>> +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, >>> + const void *crc, unsigned int crc_len) >>> +{ >>> + uint32_t expected, computed; >>> + >>> + if (crc_len != sizeof(uint32_t)) >>> + return -1; >>> + >>> + /* Stored little-endian by .long in the .rodata.*.sha section. */ >>> + expected = get_unaligned_le32(crc); >>> + computed = crc32(0, data_start, len); >>> + >>> + if (IS_ENABLED(CONFIG_DEBUG_LL)) { >>> + puts_ll("CRC "); >>> + puthexc_ll((computed >> 24) & 0xff); >>> + puthexc_ll((computed >> 16) & 0xff); >>> + puthexc_ll((computed >> 8) & 0xff); >>> + puthexc_ll(computed & 0xff); >>> + puts_ll(" vs "); >>> + puthexc_ll((expected >> 24) & 0xff); >>> + puthexc_ll((expected >> 16) & 0xff); >>> + puthexc_ll((expected >> 8) & 0xff); >>> + puthexc_ll(expected & 0xff); >>> + putc_ll('\n'); >>> + } >>> + >>> + return computed == expected ? 0 : -1; >>> +} >>> +#endif >>> + >>> void pbl_barebox_uncompress(void *dest, void *compressed_start, unsigned int len) >>> { >>> uint32_t pbl_hash_len; >>> diff --git a/scripts/gen-fw-s b/scripts/gen-fw-s >>> index 78c3193479..d7d7d15824 100755 >>> --- a/scripts/gen-fw-s >>> +++ b/scripts/gen-fw-s >>> @@ -3,13 +3,20 @@ >>> # >>> # Generate assembly source to embed firmware binary >>> # >>> -# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] >>> +# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] [verify-algo] >>> +# >>> +# verify-algo selects the metadata emitted in the .rodata.<fwstr>.sha >>> +# section, matched by firmware_ext_verify() in include/firmware.h: >>> +# sha256 (default) -- 32-byte SHA-256 >>> +# crc32 -- 4-byte CRC32 (zlib polynomial) >>> +# none -- no section, no symbols >>> >>> fwname=$1 >>> fwdir=$2 >>> secprefix=$3 >>> secflags=$4 >>> fwobjdir=$5 >>> +verify_algo=${6:-sha256} >>> >>> fwstr=$(echo "$fwname" | tr '/.-' '___') >>> fwpath="$fwdir/$fwname" >>> @@ -19,8 +26,6 @@ if [ -f "$fwpath" ]; then >>> fw_uncompressed=$(stat -c %s "$fwpath") >>> fi >>> >>> -sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') >>> - >>> echo "/* Generated by scripts/gen-fw-s */" >>> echo "#include <asm-generic/pointer.h>" >>> echo ".section .note.GNU-stack,\"\",%progbits" >>> @@ -59,19 +64,39 @@ echo ".global _fw_z_${fwstr}_end" >>> echo "_fw_z_${fwstr}_end:" >>> echo "#endif" >>> >>> -# include sha256, needed for external firmware >>> -echo " .section .rodata.${fwstr}.sha" >>> -echo " .p2align ASM_LGPTR" >>> -echo ".global _fw_${fwstr}_sha_start" >>> -echo "_fw_${fwstr}_sha_start:" >>> -echo " .byte ${sha}" >>> -echo ".global _fw_${fwstr}_sha_end" >>> -echo "_fw_${fwstr}_sha_end:" >>> -if [ -f "$fwpath" ]; then >>> - echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" >>> - echo ".error \"sha256sum invalid\"" >>> - echo ".endif" >>> -fi >>> +# verify metadata for fw-external blobs, consumed by firmware_ext_verify() >>> +# in include/firmware.h. Driven by the PBL_FIRMWARE_EXT_VERIFY Kconfig >>> +# choice (passed in from firmware/Makefile). >>> +case "$verify_algo" in >>> +none) >>> + ;; >>> +crc32) >>> + crc=$(python3 -c 'import sys,zlib; sys.stdout.write("0x%08x" % zlib.crc32(open(sys.argv[1],"rb").read()))' \ >>> + "$fwpath" 2>/dev/null || echo "0x00000000") >>> + echo " .section .rodata.${fwstr}.sha" >>> + echo " .p2align 2" >>> + echo ".global _fw_${fwstr}_sha_start" >>> + echo "_fw_${fwstr}_sha_start:" >>> + echo " .long ${crc}" >>> + echo ".global _fw_${fwstr}_sha_end" >>> + echo "_fw_${fwstr}_sha_end:" >>> + ;; >>> +sha256|*) >>> + sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') >>> + echo " .section .rodata.${fwstr}.sha" >>> + echo " .p2align ASM_LGPTR" >>> + echo ".global _fw_${fwstr}_sha_start" >>> + echo "_fw_${fwstr}_sha_start:" >>> + echo " .byte ${sha}" >>> + echo ".global _fw_${fwstr}_sha_end" >>> + echo "_fw_${fwstr}_sha_end:" >>> + if [ -f "$fwpath" ]; then >>> + echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" >>> + echo ".error \"sha256sum invalid\"" >>> + echo ".endif" >>> + fi >>> + ;; >>> +esac >>> >>> # include a string containing the firmware name. When a non existing >>> # firmware is referenced in the PBL then _fwname_${fwstr} is referenced >>> >>> base-commit: 6c70fb327d486376c1f2e37dfff2212cb9eebb1b >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >> > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig 2026-06-16 8:14 ` Ahmad Fatoum @ 2026-06-16 14:59 ` SCHNEIDER Johannes 0 siblings, 0 replies; 5+ messages in thread From: SCHNEIDER Johannes @ 2026-06-16 14:59 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox Hoi, > >Hello Johannes, > >On 6/16/26 7:46 AM, SCHNEIDER Johannes wrote: >>> On 6/13/26 9:56 PM, Johannes Schneider wrote: >>> >>>> it >>>> only re-checks the BootROM->DRAM copy against a value baked into the >>>> PBL itself, not against an external trust anchor. A 32-bit CRC over >>>> the same window catches the realistic failure mode (a corrupted copy >>>> into DRAM) at ~200x lower wall-clock cost. >>> >>> Give recent commit f2ae1a4a85d0 ("ARM: rockchip: atf: enable MMU in >>> PBL") and its follow-up a look. >>> >> >> thanks for the hint, i'm "toying" with the code now, and am wondering: instead >> of enabling the MMU to speed up the software sha256, would you have any concerns >> if i would enable and use the hardware sha256 that lives inside the imx8? >> >> from my first couple of measurements i get: >> | Variant | bl32-verify | Delta vs prev | Cumulative drop | >> | Pre-MMU-on (generic C, uncached) | 2190 ms | -- | -- | >> | Post-MMU-on (generic C, cached) | 451 ms | -1739 ms | -1739 ms | >> | Crypto-ext, per 64bytes block | 192 ms | -259 ms | -1998 ms | >> | Crypto-ext, batched | **5 ms** | -187 ms | -2185 ms | >> >> i could send either the "enable mmu" or the "enable hardware hashing" a better follow up to this patch - opinions/preferences? > >Por qué no los dos? :-) > fair question, i thought that either one or the other is relevant - measurements showed both contribute, but not in equal amounts: on imx8mP the mmu-on part is negligable, on imx8mM it is not. the batched crypto-ext is in both the biggest win. both follow up changes are now sent to the ML: [PATCH] ARM: i.MX8M: enable MMU in PBL around fw-external BL32 verify Johannes Schneider [PATCH] crypto: sha256: PBL multi-block transform via ARMv8 Crypto Extensions Crypto Extensions Johannes Schneider so please disregard this concepturally broken first attempt O:-) gruß Johannes > >>> I'd suggest you do the same: Keep the hash as-is, but set up MMU as soon >>> as you have the memory size. >>> >>> You can also play around with mimicking compression of firmware blobs >>> like we have on Rockchip. Depending on CPU vs I/O speed, this can also >>> reduce your startup time. >> >> compressing the optee tee.bin (~720KiB) and having PBL uncompress it again would >> (probably) work, i have some code that does this - but for some reason the way >> we build and sign the whole barebox binary now doesn't agree with the imx8m* >> bootrom, and it drops me back into serial-dl mode = it refuses the image >> completly. > >That's strange. You can continue booting via serial download mode and >run the bootrom command. Maybe there's some useful info there on why the >boot failed. > >> Any hints for what i need to look out for when adding such feature to the PBL? > >I haven't implemented it myself yet, but saw Sascha's patches for >Rockchip. I'd suggest you to follow that. > >Cheers, >Ahmad > >> >> >> gruß >> Johannes >> >> >>> >>> Cheers, >>> Ahmad >>> >>>> >>>> Make the verify algorithm a Kconfig choice: >>>> >>>> PBL_FIRMWARE_EXT_VERIFY_NONE no verify >>>> PBL_FIRMWARE_EXT_VERIFY_CRC32 4-byte CRC, cheap corruption check >>>> PBL_FIRMWARE_EXT_VERIFY_SHA256 current behaviour, default >>>> >>>> firmware/Makefile selects an algorithm and passes it to gen-fw-s, >>>> which emits nothing for NONE, a 4-byte .long for CRC32, or 32 bytes >>>> for SHA-256 in the per-blob .rodata.<fwstr>.sha section. The macro >>>> in include/firmware.h declares the verify symbols only when actually >>>> used (offset != 0, i.e. fw-external blobs), so NONE is a true no-op >>>> and in-tree firmware never grew a section dependency. >>>> firmware_ext_verify() dispatches via IS_ENABLED() at compile time. >>>> >>>> The verify-section symbol names (_fw_*_sha_*) are kept as-is rather >>>> than renamed, since firmware_next_image_verify() also references >>>> them for the FIRMWARE_VERIFY_NEXT_IMAGE path; under CRC32 the name >>>> no longer describes the contents, but renaming would expand the >>>> patch scope unnecessarily. >>>> >>>> Default stays at SHA-256 so existing defconfigs keep their current >>>> semantics. >>>> >>>> Assisted-by: Claude:claude-opus-4-7 >>>> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com> >>>> --- >>>> firmware/Kconfig | 33 +++++++++++++++++++++++++++ >>>> firmware/Makefile | 15 ++++++++++-- >>>> include/firmware.h | 41 +++++++++++++++++++++++---------- >>>> include/pbl.h | 2 ++ >>>> pbl/decomp.c | 35 ++++++++++++++++++++++++++++ >>>> scripts/gen-fw-s | 57 +++++++++++++++++++++++++++++++++------------- >>>> 6 files changed, 153 insertions(+), 30 deletions(-) >>>> >>>> diff --git a/firmware/Kconfig b/firmware/Kconfig >>>> index b9b4556dbd..f745819a67 100644 >>>> --- a/firmware/Kconfig >>>> +++ b/firmware/Kconfig >>>> @@ -151,4 +151,37 @@ config FIRMWARE_VERIFY_NEXT_IMAGE >>>> verified images. The function to check the next stage image hash is >>>> firmware_next_image_verify(), make sure your SoC code uses it. >>>> >>>> +choice >>>> + prompt "PBL fw-external blob verification" >>>> + default PBL_FIRMWARE_EXT_VERIFY_SHA256 >>>> + help >>>> + Algorithm used by the PBL to verify an fw-external blob against a >>>> + value embedded at build time, before handoff to TF-A. Runs with >>>> + the MMU off and DRAM reads uncached, so cost scales sharply with >>>> + blob size and algorithm. This checks integrity vs the build-time >>>> + value; it is not authentication against an external trust anchor. >>>> + >>>> +config PBL_FIRMWARE_EXT_VERIFY_NONE >>>> + bool "none" >>>> + help >>>> + Skip the verify. Appropriate when the boot chain authenticates the >>>> + blob upstream of the PBL (HAB, AHAB, signed FIT, ...) -- the >>>> + in-PBL check is then redundant. >>>> + >>>> +config PBL_FIRMWARE_EXT_VERIFY_CRC32 >>>> + bool "CRC32" >>>> + select CRC32 >>>> + help >>>> + 4-byte CRC32 (zlib polynomial). Catches accidental corruption >>>> + of the BootROM->DRAM copy; not cryptographic. >>>> + >>>> +config PBL_FIRMWARE_EXT_VERIFY_SHA256 >>>> + bool "SHA-256" >>>> + help >>>> + Cryptographic hash, software-computed with MMU off. Strongest >>>> + option but by far the slowest -- prefer NONE or CRC32 when the >>>> + blob is already authenticated upstream of the PBL. >>>> + >>>> +endchoice >>>> + >>>> endmenu >>>> diff --git a/firmware/Makefile b/firmware/Makefile >>>> index 7e433a1824..1590bed9f1 100644 >>>> --- a/firmware/Makefile >>>> +++ b/firmware/Makefile >>>> @@ -61,8 +61,19 @@ pbl-fwext-y := $(addsuffix .extgen.o, $(fw-external-y)) >>>> >>>> FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)) >>>> >>>> -filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) >>>> -filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) >>>> +# Verify algorithm passed to gen-fw-s for fw-external blobs. Drives the >>>> +# verify metadata section emitted into .extgen.S and matched by >>>> +# firmware_ext_verify() in include/firmware.h. >>>> +ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE >>>> +fw_verify_algo := none >>>> +else ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 >>>> +fw_verify_algo := crc32 >>>> +else >>>> +fw_verify_algo := sha256 >>>> +endif >>>> + >>>> +filechk_fwbin = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .rodata '' $(fwobjdir) sha256 >>>> +filechk_fwbin_ext = $(srctree)/scripts/gen-fw-s $(FWNAME) $(FIRMWARE_DIR) .pblext a $(fwobjdir) $(fw_verify_algo) >>>> >>>> $(obj)/%.gen.S: FORCE >>>> $(call filechk,fwbin) >>>> diff --git a/include/firmware.h b/include/firmware.h >>>> index 6511d56b2e..72a47f24e3 100644 >>>> --- a/include/firmware.h >>>> +++ b/include/firmware.h >>>> @@ -80,13 +80,21 @@ static inline void release_firmware(const struct firmware *fw) >>>> >>>> void firmwaremgr_list_handlers(void); >>>> >>>> -static inline void firmware_ext_verify(const void *data_start, size_t data_size, >>>> - const void *hash_start, size_t hash_size) >>>> +static inline void firmware_ext_verify(const void *data, size_t data_size, >>>> + const void *verify, size_t verify_size) >>>> { >>>> - if (pbl_barebox_verify(data_start, data_size, >>>> - hash_start, hash_size) != 0) { >>>> + int ret; >>>> + >>>> + if (IS_ENABLED(CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32)) >>>> + ret = pbl_barebox_verify_crc32(data, data_size, >>>> + verify, verify_size); >>>> + else >>>> + ret = pbl_barebox_verify(data, data_size, >>>> + verify, verify_size); >>>> + >>>> + if (ret != 0) { >>>> putc_ll('!'); >>>> - panic("hash mismatch, refusing to decompress"); >>>> + panic("firmware verify mismatch, refusing to decompress"); >>>> } >>>> } >>>> >>>> @@ -96,22 +104,31 @@ struct fwobj { >>>> void *data; >>>> }; >>>> >>>> +#if defined(CONFIG_PBL_FIRMWARE_EXT_VERIFY_NONE) >>>> +# define __fw_ext_verify(name, fwobj) do { } while (0) >>>> +#else >>>> +# define __fw_ext_verify(name, fwobj) \ >>>> + do { \ >>>> + extern char _fw_##name##_sha_start[]; \ >>>> + extern char _fw_##name##_sha_end[]; \ >>>> + firmware_ext_verify( \ >>>> + (fwobj)->data, (fwobj)->size, \ >>>> + _fw_##name##_sha_start, \ >>>> + _fw_##name##_sha_end - \ >>>> + _fw_##name##_sha_start); \ >>>> + } while (0) >>>> +#endif >>>> + >>>> #define __get_builtin_firmware(name, offset, fwobj) \ >>>> do { \ >>>> extern char _fw_##name##_start[]; \ >>>> extern char _fw_##name##_end[]; \ >>>> - extern char _fw_##name##_sha_start[]; \ >>>> - extern char _fw_##name##_sha_end[]; \ >>>> (fwobj)->data = _fw_##name##_start; \ >>>> (fwobj)->size = _fw_##name##_end - _fw_##name##_start; \ >>>> if (!(offset)) \ >>>> break; \ >>>> (fwobj)->data += (offset); \ >>>> - firmware_ext_verify( \ >>>> - (fwobj)->data, (fwobj)->size, \ >>>> - _fw_##name##_sha_start, \ >>>> - _fw_##name##_sha_end - _fw_##name##_sha_start \ >>>> - ); \ >>>> + __fw_ext_verify(name, fwobj); \ >>>> } while (0) >>>> >>>> >>>> diff --git a/include/pbl.h b/include/pbl.h >>>> index fe4367825c..57ca8b4eb3 100644 >>>> --- a/include/pbl.h >>>> +++ b/include/pbl.h >>>> @@ -29,6 +29,8 @@ fdt_device_get_match_data(const void *fdt, const char *nodepath, >>>> >>>> int pbl_barebox_verify(const void *compressed_start, unsigned int len, >>>> const void *hash, unsigned int hash_len); >>>> +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, >>>> + const void *crc, unsigned int crc_len); >>>> int pbl_load_fdt(const void *fdt, void *dest, int destsize); >>>> >>>> #define PBL_MALLOC_SIZE SZ_128K >>>> diff --git a/pbl/decomp.c b/pbl/decomp.c >>>> index 1539a6b67e..ace121eac6 100644 >>>> --- a/pbl/decomp.c >>>> +++ b/pbl/decomp.c >>>> @@ -13,6 +13,10 @@ >>>> #include <asm/sections.h> >>>> #include <pbl.h> >>>> #include <debug_ll.h> >>>> +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 >>>> +#include <crc.h> >>>> +#include <asm/unaligned.h> >>>> +#endif >>>> >>>> #define STATIC static >>>> >>>> @@ -90,6 +94,37 @@ int pbl_barebox_verify(const void *compressed_start, unsigned int len, >>>> return memcmp(hash, computed_hash, SHA256_DIGEST_SIZE); >>>> } >>>> >>>> +#ifdef CONFIG_PBL_FIRMWARE_EXT_VERIFY_CRC32 >>>> +int pbl_barebox_verify_crc32(const void *data_start, unsigned int len, >>>> + const void *crc, unsigned int crc_len) >>>> +{ >>>> + uint32_t expected, computed; >>>> + >>>> + if (crc_len != sizeof(uint32_t)) >>>> + return -1; >>>> + >>>> + /* Stored little-endian by .long in the .rodata.*.sha section. */ >>>> + expected = get_unaligned_le32(crc); >>>> + computed = crc32(0, data_start, len); >>>> + >>>> + if (IS_ENABLED(CONFIG_DEBUG_LL)) { >>>> + puts_ll("CRC "); >>>> + puthexc_ll((computed >> 24) & 0xff); >>>> + puthexc_ll((computed >> 16) & 0xff); >>>> + puthexc_ll((computed >> 8) & 0xff); >>>> + puthexc_ll(computed & 0xff); >>>> + puts_ll(" vs "); >>>> + puthexc_ll((expected >> 24) & 0xff); >>>> + puthexc_ll((expected >> 16) & 0xff); >>>> + puthexc_ll((expected >> 8) & 0xff); >>>> + puthexc_ll(expected & 0xff); >>>> + putc_ll('\n'); >>>> + } >>>> + >>>> + return computed == expected ? 0 : -1; >>>> +} >>>> +#endif >>>> + >>>> void pbl_barebox_uncompress(void *dest, void *compressed_start, unsigned int len) >>>> { >>>> uint32_t pbl_hash_len; >>>> diff --git a/scripts/gen-fw-s b/scripts/gen-fw-s >>>> index 78c3193479..d7d7d15824 100755 >>>> --- a/scripts/gen-fw-s >>>> +++ b/scripts/gen-fw-s >>>> @@ -3,13 +3,20 @@ >>>> # >>>> # Generate assembly source to embed firmware binary >>>> # >>>> -# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] >>>> +# Usage: gen-fw-s <fwname> <fwdir> <secprefix> [secflags] [fwobjdir] [verify-algo] >>>> +# >>>> +# verify-algo selects the metadata emitted in the .rodata.<fwstr>.sha >>>> +# section, matched by firmware_ext_verify() in include/firmware.h: >>>> +# sha256 (default) -- 32-byte SHA-256 >>>> +# crc32 -- 4-byte CRC32 (zlib polynomial) >>>> +# none -- no section, no symbols >>>> >>>> fwname=$1 >>>> fwdir=$2 >>>> secprefix=$3 >>>> secflags=$4 >>>> fwobjdir=$5 >>>> +verify_algo=${6:-sha256} >>>> >>>> fwstr=$(echo "$fwname" | tr '/.-' '___') >>>> fwpath="$fwdir/$fwname" >>>> @@ -19,8 +26,6 @@ if [ -f "$fwpath" ]; then >>>> fw_uncompressed=$(stat -c %s "$fwpath") >>>> fi >>>> >>>> -sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') >>>> - >>>> echo "/* Generated by scripts/gen-fw-s */" >>>> echo "#include <asm-generic/pointer.h>" >>>> echo ".section .note.GNU-stack,\"\",%progbits" >>>> @@ -59,19 +64,39 @@ echo ".global _fw_z_${fwstr}_end" >>>> echo "_fw_z_${fwstr}_end:" >>>> echo "#endif" >>>> >>>> -# include sha256, needed for external firmware >>>> -echo " .section .rodata.${fwstr}.sha" >>>> -echo " .p2align ASM_LGPTR" >>>> -echo ".global _fw_${fwstr}_sha_start" >>>> -echo "_fw_${fwstr}_sha_start:" >>>> -echo " .byte ${sha}" >>>> -echo ".global _fw_${fwstr}_sha_end" >>>> -echo "_fw_${fwstr}_sha_end:" >>>> -if [ -f "$fwpath" ]; then >>>> - echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" >>>> - echo ".error \"sha256sum invalid\"" >>>> - echo ".endif" >>>> -fi >>>> +# verify metadata for fw-external blobs, consumed by firmware_ext_verify() >>>> +# in include/firmware.h. Driven by the PBL_FIRMWARE_EXT_VERIFY Kconfig >>>> +# choice (passed in from firmware/Makefile). >>>> +case "$verify_algo" in >>>> +none) >>>> + ;; >>>> +crc32) >>>> + crc=$(python3 -c 'import sys,zlib; sys.stdout.write("0x%08x" % zlib.crc32(open(sys.argv[1],"rb").read()))' \ >>>> + "$fwpath" 2>/dev/null || echo "0x00000000") >>>> + echo " .section .rodata.${fwstr}.sha" >>>> + echo " .p2align 2" >>>> + echo ".global _fw_${fwstr}_sha_start" >>>> + echo "_fw_${fwstr}_sha_start:" >>>> + echo " .long ${crc}" >>>> + echo ".global _fw_${fwstr}_sha_end" >>>> + echo "_fw_${fwstr}_sha_end:" >>>> + ;; >>>> +sha256|*) >>>> + sha=$(sha256sum "$fwpath" 2>/dev/null | sed 's/ .*$//;s/../0x&, /g;s/, $//') >>>> + echo " .section .rodata.${fwstr}.sha" >>>> + echo " .p2align ASM_LGPTR" >>>> + echo ".global _fw_${fwstr}_sha_start" >>>> + echo "_fw_${fwstr}_sha_start:" >>>> + echo " .byte ${sha}" >>>> + echo ".global _fw_${fwstr}_sha_end" >>>> + echo "_fw_${fwstr}_sha_end:" >>>> + if [ -f "$fwpath" ]; then >>>> + echo ".if _fw_${fwstr}_sha_start + 32 - _fw_${fwstr}_sha_end" >>>> + echo ".error \"sha256sum invalid\"" >>>> + echo ".endif" >>>> + fi >>>> + ;; >>>> +esac >>>> >>>> # include a string containing the firmware name. When a non existing >>>> # firmware is referenced in the PBL then _fwname_${fwstr} is referenced >>>> >>>> base-commit: 6c70fb327d486376c1f2e37dfff2212cb9eebb1b >>> >>> -- >>> Pengutronix e.K. | | >>> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >>> >> >> > >-- >Pengutronix e.K. | | >Steuerwalder Str. 21 | http://www.pengutronix.de/ | >31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-16 15:00 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-06-13 19:56 [PATCH v1] firmware: choose PBL fw-external verify algorithm via Kconfig Johannes Schneider 2026-06-15 9:49 ` Ahmad Fatoum 2026-06-16 5:46 ` SCHNEIDER Johannes 2026-06-16 8:14 ` Ahmad Fatoum 2026-06-16 14:59 ` SCHNEIDER Johannes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox