From 58a50a499d20ef567415c61e2e3d463b134ab423 Mon Sep 17 00:00:00 2001 From: Tim Gover Date: Tue, 27 Oct 2026 15:32:57 +0000 Subject: [PATCH 1/3] rpiboot: Leave room for bootfiles.bin in the executable directory The directory next to the executable could fill PATH_MAX, which would truncate bootfiles.bin when it is appended to form bootfiles_path. Size exe_msg_dir so that the full path always fits. get_exe_msg_dir already rejects a directory that is too long. This also fixes a gcc -Wformat-truncation warning at -O1 and above. --- main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.c b/main.c index 15827b4..c197369 100644 --- a/main.c +++ b/main.c @@ -37,7 +37,7 @@ int loop = 0; int overlay = 0; long delay = 500; char * directory = NULL; -char exe_msg_dir[PATH_MAX]; +char exe_msg_dir[PATH_MAX - sizeof("bootfiles.bin") + 1]; char * metadata_path = NULL; char pathname[64] = {0}; char * targetpathname = NULL; From b8cf25a352eed415d0267390d665c0917711f58d Mon Sep 17 00:00:00 2001 From: Tim Gover Date: Tue, 27 Oct 2026 15:09:56 +0000 Subject: [PATCH 2/3] test: Add bootfiles.bin and file lookup tests Add a make check target which builds host tests against main.c, bootfiles.c and decode_duid.c without needing a USB device. The tests cover bootfiles.bin extraction from valid and corrupt archives, the order in which the file server searches bootfiles.bin, the chip and port directories and the embedded msd files, path traversal, FACTORY_UUID decoding, and that each shipped bootfiles.bin contains the files that BCM2711 and BCM2712 load. --- .gitignore | 2 + Makefile | 6 +- test/Makefile | 18 +++ test/README.md | 9 ++ test/test_files.c | 304 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 test/Makefile create mode 100644 test/test_files.c diff --git a/.gitignore b/.gitignore index 71be6c7..055a979 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ bin2c *.exe *.swp temp/ +test/test_files +test/test_files.tmp/ diff --git a/Makefile b/Makefile index b8b1fcc..18324af 100755 --- a/Makefile +++ b/Makefile @@ -48,7 +48,11 @@ uninstall: rm -f $(DESTDIR)$(INSTALL_PREFIX)/bin/rpiboot rm -rf $(DESTDIR)$(INSTALL_PREFIX)/share/rpiboot +check: + $(MAKE) -C test check + clean: rm -f rpiboot msd/*.h bin2c + $(MAKE) -C test clean -.PHONY: uninstall clean +.PHONY: uninstall clean check diff --git a/test/Makefile b/test/Makefile new file mode 100644 index 0000000..e25f033 --- /dev/null +++ b/test/Makefile @@ -0,0 +1,18 @@ +TOP = .. +SCRATCH = test_files.tmp + +check: test_files + rm -rf $(SCRATCH) && mkdir $(SCRATCH) + ./test_files $(SCRATCH) $(TOP) + rm -rf $(SCRATCH) + +test_files: test_files.c $(TOP)/main.c $(TOP)/bootfiles.c $(TOP)/decode_duid.c $(TOP)/msd/bootcode.h $(TOP)/msd/start.h + $(CC) -Wall -Wextra -g $(CPPFLAGS) $(CFLAGS) -o $@ test_files.c $(TOP)/bootfiles.c $(TOP)/decode_duid.c `pkg-config --cflags --libs libusb-1.0` -DGIT_VER=\"test\" -DPKG_VER=\"test\" -DBUILD_DATE=\"test\" -DDEFAULT_MSG_DIR=\"/nonexistent/\" $(LDFLAGS) + +$(TOP)/msd/%.h: + $(MAKE) -C $(TOP) msd/$*.h + +clean: + rm -rf test_files $(SCRATCH) + +.PHONY: check clean diff --git a/test/README.md b/test/README.md index 6f47927..87288ba 100644 --- a/test/README.md +++ b/test/README.md @@ -2,3 +2,12 @@ ## validate-hsm-wrapper Verifies that the HSM wrapper (example) script and the direct private key API produce identical results. + +## test_files +Host tests for `bootfiles.bin` extraction, the file lookup order used by the +file server, the shipped `bootfiles.bin` archives and FACTORY_UUID decoding. +No USB device is required. +``` +make check +make check CC=clang CFLAGS="-fsanitize=address,undefined -fno-sanitize-recover=all" +``` diff --git a/test/test_files.c b/test/test_files.c new file mode 100644 index 0000000..c94c018 --- /dev/null +++ b/test/test_files.c @@ -0,0 +1,304 @@ +// Host tests for bootfiles.bin extraction, file lookup and DUID decoding. +// main.c is included directly so that the static file lookup state is +// visible to the tests. No USB device is required. +// +// Usage: test_files + +#define main rpiboot_main +#include "../main.c" +#undef main + +#include +#include + +#define TAR_BLOCK 512 + +static int failures; +static char scratch[256]; + +#define CHECK(cond) do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ +} while (0) + +static void tar_add(FILE *fp, const char *name, const char *size_field, + const void *data, size_t len) +{ + unsigned char hdr[TAR_BLOCK] = {0}; + static const unsigned char pad[TAR_BLOCK]; + + memcpy(hdr, name, strnlen(name, 100)); + snprintf((char *) hdr + 124, 12, "%s", size_field); + fwrite(hdr, 1, sizeof(hdr), fp); + fwrite(data, 1, len, fp); + fwrite(pad, 1, (TAR_BLOCK - (len % TAR_BLOCK)) % TAR_BLOCK, fp); +} + +static void tar_add_file(FILE *fp, const char *name, const char *data) +{ + char size[24]; + + snprintf(size, sizeof(size), "%011lo", (unsigned long) strlen(data)); + tar_add(fp, name, size, data, strlen(data)); +} + +static void tar_end(FILE *fp) +{ + static const unsigned char zero[2 * TAR_BLOCK]; + + fwrite(zero, 1, sizeof(zero), fp); + fclose(fp); +} + +static FILE *create(const char *fmt, const char *name) +{ + char path[PATH_MAX]; + FILE *fp; + + snprintf(path, sizeof(path), fmt, scratch, name); + fp = fopen(path, "wb"); + if (!fp) { + perror(path); + exit(1); + } + return fp; +} + +static void write_file(const char *name, const char *data) +{ + FILE *fp = create("%s/%s", name); + + fputs(data, fp); + fclose(fp); +} + +static void make_dir(const char *name) +{ + char path[PATH_MAX]; + + snprintf(path, sizeof(path), "%s/%s", scratch, name); + if (mkdir(path, 0755) && errno != EEXIST) { + perror(path); + exit(1); + } +} + +// Read the whole of fp into buf as a string and close it +static const char *slurp(FILE *fp) +{ + static char buf[256]; + size_t n; + + if (!fp) + return NULL; + n = fread(buf, 1, sizeof(buf) - 1, fp); + buf[n] = 0; + fclose(fp); + return buf; +} + +static int same(const char *a, const char *b) +{ + return a && b && !strcmp(a, b); +} + +static int archive_has(const char *archive, const char *name, const char *data) +{ + unsigned long size = 0; + unsigned char *buf = bootfiles_read(archive, name, &size); + int ok = buf && (!data || (size == strlen(data) && !memcmp(buf, data, size))); + + free(buf); + return ok; +} + +static void test_bootfiles_read(void) +{ + char archive[PATH_MAX]; + char name[101]; + FILE *fp; + + snprintf(archive, sizeof(archive), "%s/good.tar", scratch); + fp = create("%s/%s", "good.tar"); + tar_add_file(fp, "2712/", ""); + tar_add_file(fp, "2712/config.txt", "cfg"); + tar_add_file(fp, "2712/Bootcode5.bin", "boot5"); + tar_end(fp); + + CHECK(archive_has(archive, "2712/config.txt", "cfg")); + CHECK(archive_has(archive, "2712/bootcode5.bin", "boot5")); + CHECK(!archive_has(archive, "2711/config.txt", NULL)); + CHECK(!archive_has(archive, "config.txt", NULL)); + + snprintf(archive, sizeof(archive), "%s/missing.tar", scratch); + CHECK(!archive_has(archive, "2712/config.txt", NULL)); + + // Size field claims more data than the archive holds + snprintf(archive, sizeof(archive), "%s/oversize.tar", scratch); + fp = create("%s/%s", "oversize.tar"); + tar_add(fp, "2712/config.txt", "00000100000", "cfg", 3); + tar_end(fp); + CHECK(!archive_has(archive, "2712/config.txt", NULL)); + + // Archive truncated part way through a header + snprintf(archive, sizeof(archive), "%s/short.tar", scratch); + fp = create("%s/%s", "short.tar"); + tar_add_file(fp, "2712/config.txt", "cfg"); + fwrite("2712/bootcode5.bin", 1, 18, fp); + fclose(fp); + CHECK(!archive_has(archive, "2712/bootcode5.bin", NULL)); + + // A 100 character name has no terminator in the header + memset(name, 'a', 100); + name[100] = 0; + snprintf(archive, sizeof(archive), "%s/longname.tar", scratch); + fp = create("%s/%s", "longname.tar"); + tar_add_file(fp, name, "long"); + tar_end(fp); + CHECK(!archive_has(archive, name, NULL)); + name[99] = 0; + CHECK(archive_has(archive, name, "long")); +} + +static void test_check_file(void) +{ + char dir[512]; + FILE *fp; + + snprintf(dir, sizeof(dir), "%s/msg", scratch); + make_dir("msg"); + make_dir("msg/2711"); + make_dir("msg/2712"); + make_dir("msg/1-2"); + fp = create("%s/msg/%s", "bootfiles.bin"); + tar_add_file(fp, "2712/bootcode5.bin", "tar-boot5"); + tar_add_file(fp, "2712/config.txt", "tar-config"); + tar_end(fp); + write_file("msg/2712/config.txt", "dir-config"); + write_file("msg/2711/start4.elf", "dir-2711-start4"); + write_file("msg/start4.elf", "dir-start4"); + write_file("msg/boot.img", "dir-boot"); + write_file("msg/1-2/start4.elf", "port-start4"); + write_file("msg/1-2/bootcode4.bin", "port-boot4"); + write_file("msg/2711/bootcode4.bin", "dir-2711-boot4"); + + // BCM2712 with bootfiles.bin + bcm2711 = 0; + bcm2712 = 1; + use_bootfiles = 1; + snprintf(bootfiles_path, sizeof(bootfiles_path), "%s/bootfiles.bin", dir); + CHECK(same(slurp(check_file(dir, "bootcode5.bin", 1)), "tar-boot5")); + CHECK(same(slurp(check_file(dir, "config.txt", 1)), "dir-config")); + CHECK(same(slurp(check_file(dir, "boot.img", 1)), "dir-boot")); + CHECK(!check_file(dir, "../msg/boot.img", 1)); + CHECK(!check_file(dir, "missing", 1)); + CHECK(!check_file(NULL, "missing", 1)); + + // BCM2711 from a directory, chip subdirectory first + bcm2711 = 1; + bcm2712 = 0; + use_bootfiles = 0; + CHECK(same(slurp(check_file(dir, "start4.elf", 1)), "dir-2711-start4")); + CHECK(same(slurp(check_file(dir, "boot.img", 1)), "dir-boot")); + + // Port overlay takes priority except for the second stage + overlay = 1; + strcpy(pathname, "1-2"); + CHECK(same(slurp(check_file(dir, "start4.elf", 1)), "port-start4")); + CHECK(same(slurp(check_file(dir, "bootcode4.bin", 1)), "dir-2711-boot4")); + overlay = 0; + pathname[0] = 0; + + // BCM2835/6/7 fall back to the embedded msd files + bcm2711 = 0; + fp = check_file(NULL, "bootcode.bin", 1); + CHECK(fp && fgetc(fp) == msd_bootcode_bin[0]); + if (fp) { + fseek(fp, 0, SEEK_END); + CHECK(ftell(fp) == (long) msd_bootcode_bin_len); + fclose(fp); + } + fp = check_file(NULL, "start.elf", 1); + CHECK(fp != NULL); + if (fp) + fclose(fp); + CHECK(!check_file(NULL, "bootcode.bin", 0)); + CHECK(!check_file(NULL, "config.txt", 1)); +} + +// Files that the second stage on each chip loads from bootfiles.bin +static void test_shipped_bootfiles(const char *top) +{ + static const char *archives[] = { + "firmware", "mass-storage-gadget64", "rpi-imager-embedded", + "secure-boot-example", + }; + static const char *files[] = { + "bootmain", "mcb.bin", "memsys00.bin", "memsys01.bin", + "memsys02.bin", "memsys03.bin", + }; + static const struct { const char *chip, *second_stage; } chips[] = { + { "2711", "bootcode4.bin" }, + { "2712", "bootcode5.bin" }, + }; + char archive[PATH_MAX], name[64]; + unsigned long size; + unsigned char *buf; + unsigned a, c, f; + + for (a = 0; a < sizeof(archives) / sizeof(archives[0]); a++) { + snprintf(archive, sizeof(archive), "%s/%s/bootfiles.bin", top, archives[a]); + for (c = 0; c < sizeof(chips) / sizeof(chips[0]); c++) { + for (f = 0; f <= sizeof(files) / sizeof(files[0]); f++) { + snprintf(name, sizeof(name), "%s/%s", chips[c].chip, + f ? files[f - 1] : chips[c].second_stage); + size = 0; + buf = bootfiles_read(archive, name, &size); + if (!buf || !size) + fprintf(stderr, "FAIL %s: %s missing\n", archive, name); + failures += !buf || !size; + free(buf); + } + } + } +} + +static void test_duid(void) +{ + char out[DUID_LENGTH + 1]; + char in[128]; + + // "ABC" = (14 * 1600) + (15 * 40) + 16 + 1 + strcpy(in, "59e9"); + CHECK(duid_decode_c40(in, out) == 0 && same(out, "ABC")); + strcpy(in, "59e9_59e959e9"); + CHECK(duid_decode_c40(in, out) == 0 && same(out, "ABCABCABC")); + strcpy(in, "59e959e9_59e959e9_59e959e9_59e959e9_59e959e9_59e959e9"); + CHECK(duid_decode_c40(in, out) == 0 && strlen(out) == DUID_LENGTH); + strcpy(in, "59e959e9_59e959e9_59e959e9_59e959e9_59e959e9_59e959e9_59e9"); + CHECK(duid_decode_c40(in, out) == -1); + strcpy(in, "1"); + CHECK(duid_decode_c40(in, out) == -1); + strcpy(in, ""); + CHECK(duid_decode_c40(in, out) == 0 && same(out, "")); +} + +int main(int argc, char *argv[]) +{ + if (argc != 3) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + snprintf(scratch, sizeof(scratch), "%s", argv[1]); + + test_bootfiles_read(); + test_check_file(); + test_shipped_bootfiles(argv[2]); + test_duid(); + + free(bootfile_data); + printf("%s: %d failures\n", argv[0], failures); + return failures != 0; +} From 4f439e69ea48e22fb87a6e7800d4c8fe3e566ea5 Mon Sep 17 00:00:00 2001 From: Tim Gover Date: Tue, 27 Oct 2026 15:10:02 +0000 Subject: [PATCH 3/3] ci: Run the host tests with ASan and UBSan Build and run make check with clang and the address and undefined behaviour sanitisers so that memory errors in the bootfiles.bin and file lookup code fail the pull request. --- .github/workflows/static-analysis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 5217d72..0545c43 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -34,3 +34,6 @@ jobs: -DGIT_VER='"ci"' -DPKG_VER='"ci"' -DBUILD_DATE='"ci"' \ -DDEFAULT_MSG_DIR='"/usr/share/rpiboot/mass-storage-gadget64/"' \ main.c bootfiles.c decode_duid.c + + - name: Run host tests with ASan and UBSan + run: make check CC=clang CFLAGS="-Werror -fsanitize=address,undefined -fno-sanitize-recover=all"