cbwww/site/news/audit2.md

% Canoeboot Build System Audit 2 % Leah Rowe % 19 July 2024

Heavy amount of code reduction in this audit, and general cleanup. A new Canoeboot release is planned, for the early days of August 2024.

Introduction

Canoeboot is a free/opensource boot firmware project. It replaces your proprietary BIOS/UEFI firmware, on supported x86 and ARM computers. It does this by providing an automated build system to download, patch and compile the various upstream sources (e.g. coreboot, GRUB, SeaBIOS). Coreboot is used for hardware initialisation, configuring everything from your CPU, memory controller all way to peripherals, readying the hardware so that it can run software, e.g. GNU+Linux and BSD systems. You can essentially think of cbmk, which is Canoeboot's build system, as a source-based package manager. It is what the Canoeboot releases are built with. The cbmk build system essentially implements a coreboot distro, the same way you might think of a GNU+Linux distribution; it systematically downloads, resets (to specific revisions) and patches various upstream project sources such as coreboot, U-Boot and GRUB, automatically building entire coreboot images. This build system is what creates Canoeboot release images and source tarballs.

Extensive auditing has been performed on cbmk, since the Canoeboot 20240612 release. These audits fix bugs, reduce code bloat and generally improve the efficiency of cbmk, adding and removing features in a careful, conservative way, with a focus on clean code.

Remember the magic words: code equals bugs. Fewer lines of code will yield fewer bugs.

This article covers changes from Canoeboot 20240612, up to revision 16f9ad55ca2ef7d19d10e78b0ff9e1c84bfc4fc1 from 19 July 2024.

You can read about the previous audit in the article for Canoeboot Build System Audit 1.

Notable code size reduction

There are 765 lines of shell script in the build system, versus 1054 in the Canoeboot 20240612 release. Canoeboot's build system is written entirely in POSIX sh; not BASH, not KSH, not ZSH, jush sh! It is the build system of build systems; the design focuses on re-using (read: abusing) the build systems of the projects it uses, avoiding as much bloat as possible. More information is available in the cbmk build system documentation.

This is a difference of 289 lines, or a 28% reduction. The main focus has been on removing legacy bloat from the build system, vastly increasing the overall code cleanliness and efficiency. Several new safety and redundancy features were added, such as auto-rebuild when configs change(and auto-redownload) for sources, and additional redundant handling of both repositories and file downloads, cached during operation to avoid over-use of internet bandwidth.

Plan for next release

This code reduction has been the main focus; the focus now, at the next release, will be new payloads and boards, in addition to testing newer upstream revisions of projects such as coreboot, on every machine supported by Canoeboot. A release is planned for early August 2024.

A lot of work on new ports is planned. There are a number of new mainboards that will be available, in the next Canoeboot release.

Summarised list of changes

The most interesting changes are marked in bold. "Interesting" means that the change greatly improves the usefulness/reliability of Canoeboot, or that it affects the user in a profound and noticeable way.

Changes are in order per category, from newest to oldest:

Feature changes

Some unused features have been removed, and yet more added. The overall focus of Audit 2 has been to remove legacy cruft from cbmk, and in general to simplify the operation of cbmk, Canoeboot's build system. Several new safety features were implementated for this audit, in addition to features that generally improve the redundancy of certain operations, and overall reliability in cbmk.

The changes are as follows:

• Put the cache directory location in an environmental variable, named XBMK_CACHE. If unset, it defaults to $PWD/cache, where PWD is the current location that the instance of cbmk is running from, otherwise if defined, the location is used. This means that the same cached files are re-used on the cbmk copy created when running ./update release, thus saving on a lot of internet bandwidth if the main instance already has a lot of files cached - it also means that you can re-use another cache from elsewhere, like if you have multiple clones of cbmk on your system.
• Allow using coreboot's own build system, to build payloads. Canoeboot's build system builds and adds payloads, with coreboot configurations enabling CONFIG_PAYLOAD_NONE exclusively. However, advanced users may wish to use something else such as Tianocore, which Canoeboot may/will not provide (with Tianocore it's will not). Simply set build_depend="" in the target.cfg file for a given mainboard, and then enable a payload under coreboot's menuconfig interface, or by direct modification of the defconfig file. When CONFIG_PAYLOAD_NONE is not set, cbmk will skip adding a payload, because it's a given that then coreboot's own build system would have done it. Canoeboot will always enable CONFIG_PAYLOAD_NONE on all official targets, so this feature is only intended for power users.
• Unified cache file handling for files defined as git submodules, fetching for specific projects. They are now cached within the same base directory.
• script/trees: Automatically re-download and re-build a given project, when configuration changes, on any file under specific directories in config/ pertaining to it; this is done by concatenating the sha512sum hashes for each of those files, and then making a sha512sum hash from that. The hash is stored, and compared to the current one; if it differs, then you know something changed. This feature does not track modifications to the actual downloaded source code for a given project, because this change also tracks the patches/ directories; direct modification (by the user) of the source is not assumed, under the cbmk model, but checking for this may be possible. For example, run the same concatenation logic on a file containing the output of commands such as git describe and git status, perhaps git diff. It was decided that the latter would be undesirable, because developers (of Canoeboot, working with cbmk) may want to experiment with quick changes, before preparing patch files to go under config/. - the user previously had to know which files and directories to manually remove, when re-running build commands after modifying a given project within cbmk. Now it's automatic.
• include/git.sh: Don't download multi-tree projects to src/project/project, e.g. src/coreboot/coreboot, when downloading the upstream repository, for later copying to the actual destination e.g. src/coreboot/default - instead, use the same cached location as alluded to in the two changes below. This avoids needing to delet those directories, when preparing release archives.
• include/git.sh: Complementing the change seen below, also cache files universally, for submodules files (e.g. crossgcc files). This avoids wasting bandwidth when building for multiple coreboot trees, in cases where multiple trees share the same URLs for crossgcc tarballs.
• include/git.sh: Cache all git downloads, reset to HEAD, before copying to the final location; this is also done on submodules. If a cached repository exists, run git pull on it but without an exit under fault conditions; if a fault occurs, including when trying to reset on a copy of it, fall back to the old logic where the repository is downloaded (online) directly to the destination. This change results in substantially reduced internet bandwidth usage, especially when downloading submodules for multiple coreboot trees, in cases where those trees share common revisions.
• script/trees: Support -d (flag) for custom build logic; it does the same thing as -b, but with a variable named dry being set to :; this is prefixed on most build-related commands, and can be further prefixed as desired, in mkhelper functions. The result is that you can do dry runs on a given build. This is used, to provide building of coreboot utilities, without needing specific hacks to be hardcoded in script/trees. The ./update trees -b coreboot TREE utils command is no longer available; instead, do ./update trees -d coreboot TREE. The result of -d, combined with use of mkhelper functions, permits very flexible use of the trees script, essentially allowing it to be extended on a plugin-type infrastructure, in any number of ways. The reason for implementing this is precisely so that project-specific hacks can be removed; the trees script now contains much more generalised logic, and project-specific logic was moved to mkhelper functions.
• script/trees: A premake variable is supported in mkhelper.cfg files, defining a function to be executed before running make, on a given project.
• script/trees: A postmake variable is supported in mkhelper.cfg and target.cfg files, defining a function to be executed immediately after running make, on a given project. This is used during coreboot builds, to add payloads on a given coreboot image. It can be used for any other purpose. More specifically: postmake is executed after handling the defconfig file entirely, rather than just after running make; the latter is handled by mkhelper.
• script/trees: Support build dependencies, complementing the existing fetch dependencies. The build_depend variable can define which other sources need to be built, before building the current target. This is being used on coreboot targets, to manually and surgically specify which GRUB and SeaBIOS trees should be compiled, per coreboot target. A fallback declaration is specified in config/data/coreboot/mkhelper.cfg, for cases where a target.cfg file does not specify it. This change means that certain hardcoded build dependency logic was then removed, instead relying on this much more generalised build_depend logic.
• script/trees: Allow global configuration on multi-tree projects, via file config/data/PROJECT/mkhelper.cfg, e.g. config/data/coreboot/mkhelper.cfg. This is loaded before target.cfg. That way, global configuration can be provided and then overridden.
• script/trees: If a valid flag is passed, but without argument, a loop is now executed with that flag on every defined project, both single- and multi-tree. For example, if -f is passed, it will download every project. If -b is passed, it will build every project. When it passes over multi-tree projects, it operates on every tree within that project. This fact enabled other parts of the build system to be simplified; for example, the logic in mkrelease() no longer needs to implement the same kind of loop, instead simply running ./update trees -f to download every project and tree. Similar changes have been made, on that same basis.
• script/trees: Support use of a variable named mkhelper, first defined in project target.cfg files. This variable defines the name of a function, to be called immediately after running make for that project, under the run_make_command() function. This was initially implemented, so that GRUB's grub-mkstandalone utility could be called from the trees script, but in a way that doesn't hardcode such execution. This is because part of the audit's main focus is to remove project-specific hacks, generalising a lot more logic. Prior to Audit 2, the trees script contained a lot of hacks for specific projects, notably coreboot and GRUB, to compile them properly.
• script/trees: Only permit one single-tree project to be built, where an argument is provided. This script previously supported building multiple single-tree projects, defined by argument, but it was quite error prone and there's no clean way to otherwise do it. We don't use the script this way, anywhere in cbmk, and users are advised the same.
• script/roms: Only Support SeaBIOS and SeaGRUB, on x86 mainboards. SeaGRUB is a configuration whereby SeaBIOS starts first, but immediately tries to load GRUB from the flash. This complements the other change, listed below. We will no longer provide configurations where GRUB is the primary payload, precisely to mitigate the same issue as described below (cbmk issue 216). If GRUB is enabled, on a given mainboard, SeaBIOS-only setups are not provided; only SeaGRUB is provided. You can press ESC in the SeaGRUB menu, to access other boot methods besides GRUB from flash, so you can use it in the same way; additionally, you can remove the bootorder file from CBFS if you wish, or change the bootorder file, in such a way that GRUB does not load first. For example, you might want a BSD installation, and BSD bootloaders should be loaded from SeaBIOS directly. This can be done as follows: cbfstool canoeboot.rom remove -n bootorder - then flash it. The SeaBIOS code is much smaller and more robust than GRUB, so it's a safe choice of primary payload, and it's extremely unlikely that SeaBIOS would ever break.
• script/roms: Removed support for grubonly SeaBIOS. This was a feature where SeaBIOS would try to load GRUB first (from CBFS), but also disable the SeaBIOS menu. We still provide SeaGRUB, but the SeaBIOS menu is always accessible now. This is to mitigate future issues similar to lbmk issue 216. The Canoeboot 20240612 already removed actual configs using grubonly, and we will no longer provide them in Canoeboot, so this feature in cbmk became legacy cruft that can simply be removed. The idea is simple: safety first. This has implications for GRUB hardening; you can insert the correct bootorder file, to once again disable SeaBIOS, but you must first verify that GRUB is stable. The GRUB hardening guide already documents how to do this.
• script/roms: Don't insert timeout.cfg. Instead, hardcode the GRUB timeout to a reasonable default (8 seconds). You can still insert your own timeout config as before, and GRUB will honour it.
• script/roms: Insert the background image in GRUB's memdisk, instead of coreboot's CBFS. This reduces the amount of code, because we use it as an argument on the grub-mkstandalone command, and it means that the GRUB images can be used standalone in other build systems. The image is very small, so it's OK to do it this way.
• script/roms: Removed extra checks on grub_scan_disk, because we no longer allow it to be overridden, instead relying only on target.cfg files, which are carefully crafted per board. This setting configures the order of which types of devices Canoeboot's grub.cfg scans first, between device types: PCI-E/NVMe, SATA/AHCI and IDE/ATA. Mate Kukri added NVMe SSD support no GRUB, in the Canoeboot 20240612 release.
• script/roms: Removed dangerous -p, -s and -d options. Due to lbmk issue 216 (GRUB memory issue) affecting Libreboot 20240504, several safety mitigations have been made, for anti-brick purposes. These options enabled use of a custom payload and keymap, which meant that users were able to more easily create a setup affected by lbmk issue 216. See: https://codeberg.org/libreboot/lbmk/issues/216 - this was fully mitigated, by virtue of lbmk reconfiguration, but the issue itself is not yet fixed as of the day of today's announcement, the one you're reading now, so it's still possible to create such a broken setup. Canoeboot mitigates this fact, by avoiding such configurations. NOTE: This issue never affected Canoeboot, but Canoeboot keeps in sync with Libreboot where possible.

Configuration changes

This pertains to anything under config/, for any changes that are of note, but it does not pertain to revisions for specific projects, nor does it cover specific patches applied to projects; the latter will be covered in other sections of this audit announcement.

The changes are as follows:

• coreboot: Set postmake in config/data/coreboot/mkhelper.cfg, defining a function that adds payloads to the coreboot image that was just built. This mkhelper config also integrates include/rom.sh, containing these functions. This replicates the functionality originally provided by script/roms.
• coreboot: Set build_depend on target.cfg files for specific mainboards. This is used to manually specify which GRUB and SeaBIOS trees should be compiled, required when compiling for a specific target, for the next stage where a payload is added to the coreboot image, because cbmk does not use coreboot's own payload logic, instead building and adding everything itself (the coreboot configs all enable CONFIG_PAYLOAD_NONE). For cases where a target.cfg file does not specify mkhelper, a fallback default entry is dictated within config/data/coreboot/mkhelper.cfg.
• GRUB: Only load the xhci module from grub.cfg; the install_modules variable now defines xhci, instead of modules defining it. This means that the xHCI module is no longer automatically loaded during GRUB machine initialisation, but it's then loaded immediately when the GRUB shell starts, and the grub.cfg file is first parsed, then executed. This mitigates a previous issue where booting into GRUB with a USB device connected sometimes made GRUB hang; this issue affected Canoeboot 20240612, on Haswell thinkpads, and revised images (and a revised source tarball) was then issued for that release, containing the very fix described here. NOTE: This does not actually affect any Canoeboot users, because the xhci tree of GRUB is not currently used in Canoeboot releases, but it is maintained in sync with lbmk.
• coreboot: Define makeargs in config/data/coreboot/mkhelper.cfg. It was not practical to define this, and other settings, in each target.cfg file due to how many targets there are, but the mkhelper feature makes this much easier. To mitigate the previous issue, the trees script hardcoded coreboot makeargs in a special variable, cbmakeargs; this variable has since been removed, because it's no longer used.
• GRUB: Define common build options in config/data/grub/mkhelper.cfg, instead of repeating them in target.cfg files for each GRUB tree. The mkhelper.cfg feature was added, defined in the Features section above, which is loaded per-project on multi-tree projects, before each target file. It allows easier configuration tree-wide on multi-tree projects.

Bug fixes

There are fewer acute bug fixes in Audit 2, because bugfixes was the primary focus of Audit 1. The word acute is used, referring to triggered bugs, because many of these changes are pre-emptive bug fixes, preventing future bugs that have been foreseen, based on future modification to existing logic. The focus of Audit 2 has been general code reduction, feature simplification and improvements to safety features / error handling (many pre-emptive bugfixes implement newer safety features, but are listed as pre-emptive fixes, not features).

The changes are as follows:

• script/trees: Hardcode makeargs for crossgcc specifically, by using another variable (within that script) called xgccargs. This prevents the build threads being default (one thread), ensuring that we set it to the value as defined by XBMK_THREADS. The bug was triggered when building the U-Boot images, because U-Boot does not specify CPUS= on makeargs.
• include/git.sh: Re-try git pull three times, when updating the cache Git repository for a given project or submodule. This mitigates possible instability in the user's internet connection, where it's online but may otherwise experience jitter (the same issue could also occur on the internet connection hooking up whichever server cbmk is interacting with, such as the coreboot web server).
• U-Boot (patch): Courtesy of Alper Nebi Yasak, a mitigation is provided in U-Boot working around the lack of support for clock timings on eDP displays; the clock setting was removed in the devicetree, because display works without it, but with it, display breaks because drivers do not yet support it.
• U-Boot (patch): Courtesy of Alper Nebi Yasak, a fix was made that prevents breaking the build process for U-Boot, when building binman images. In cbmk, we do not use the BL31 firmware from U-Boot itself, because coreboot does this, so it's patched out in Canoeboot and we defer to coreboot's logic.
• script/trees: Don't continue, when no argument is given. main() used to be the only funsction that runs, in this script, but now we source a config file afterward, and run the build after that. Whan a flag is provided without OPTARG, this means that we are continuing such action in error; in practise, it meant that nothing was done because the command was set by mkhelpercfg and cmd, which would be empty in this situation, but it might cause issues depending on the sh implementation, so this fix is pre-emptive.
• include/: Remove +x permissions on these files, because they must never be executed directly; they are stubbed from build and script/trees. This is listed as a bug fix, because having executable permission on these files is a potential security issue, depending on how cbmk might be modified in the future, so it's a pre-emptive fix.
• include/rom.sh: Only build coreboot utilities before building coreboot itself. This prevents coreboot.rom from being wiped out, when building it, due to idiosyncrasis in coreboot's own build system. This is a pre-emptive fix, because the issue didn't really cause actual trouble in practise, except under certain test conditions that are unlikely in real use.
• include/lib.sh: Much stricter checks in chkvars(); the variable string may have been set, but otherwise empty. This function was only checking +x via variable expansion, which only yields negative if the variable was never declared. The intention is to check both unitialised and empty strings, per variable. This is a pre-emptive bug fix, because this bug did not seemingly cause any issues in practise, but a bug is still a bug.
• script/roms: Made U-Boot be built before checking ubootelf, instead of after. The check was flawed, because if U-Boot didn't exist, but a given build would be vzerified by the first check, the check would still fail after build, because the check works by first checking the existence of a given U-Boot build file, deferring to another and then exiting if that doesn't exist, with error status. This is a pre-emptive bug fix, because this bug wasn't triggered in practise since actual targets already used the fallback build, not the first one. Therefore, this fix is pre-emptive for future U-Boot targets in cbmk.
• include/lib.sh: Do not exceed 80 characters on the line that checks the versiondate variable. Such excess is considered a bug.
• script/trees: Exit with error status if target.cfg not provided, on multi-tree projejcts. It was already by design that this should occur, but we previously did not support target.cfg files on single-tree projects. Single-tree target.cfg support was later added, done by making target.cfg optional there, but the change accidentally made it optional on multi-tree projects. In practise, all multi-tree projects included this file per target, so this is a pre-emptive bug fix on the theory that future maintainers might accidentally exclude target.cfg on a given tree, within a multi-tree project.
• build: Don't delete TMPDIR if it's /tmp. We set it to a subdirectory under /tmp, and subsequent mktemp calls create resources under it, but it's possible in the future that this functionality might be broken; therefore, this is a pre-emptive bug fix, on the (correct) theory that code equals bugs, and that regressions can and will occur in the future. One could say that this is a safety feature; it could just as easily be listed on Feature changes. This is more strongly a pre-emptive bug fix, as opposed to an actual feature. Upon exit from the main parent instance of cbmk, TMPDIR is deleted. It is set to a subdirectory of /tmp, precisely so that we have a unified directory location for all temporary files, which then get deleted in bulk (upon exit).
• script/trees: Explicitly err if OPTARG is not set, for a given flag. We otherwise rely on -e, but different sh implementations may behave differently. Therefore, this is a pre-emptive bug fix, on the theory that execution may otherwise continue erroneously (depending on the implementation). NOTE: this is only listed for documentary reasons, but the script was later modified to allow a lack of argument, whereby the script would be re-called recursively on all projects with the exact same flag, while defining an argument throughout.
• script/trees: Call err if multiple flags are provided. The script is only designed for one flag. Exit with error status if multiple flags are provided.
• script/trees: Explicitly set mode for flag -b, rather than leaving it at the default state (empty). This is to mitigate the possibility (from later code changes) where other flags might set the string. We need the string to be empty, when the -b flag is used; this is a pre-emptive bug fix, fixing a bug that doesn't actually exist, but rather, preventing such a bug later on.
• include/lib.sh: Make the presence of a projectname file mandatory, with the project name inside. This prevents the theoretical issue where a project is nameless, because this file is used during the build process when naming release tarballs; theoreticaly, because in practise, cbmk would never remove this file, but the build system is designed to be as adaptable as possible.
• script/trees: If the crossgcc build fails, run it again first, before returning with error status. If the second build passes, don't yield an error. Sometimes the crossgcc build can fail for whatever reason, but a second run will succeed. This avoids having to re-run the rest of cbmk, just to re-run the crossgcc build. In most cases, the first attempt passes. Coreboot's own buildgcc script is quite complex, and error-prone.
• script/trees: Don't check if crossgcc is already built. The check was done based on whether the xgcc directory exists, but coreboot's build system may have exited with error status before, having not completed all builds. The result was that subsequent runs would result in a permanently broken crossgcc build, unless the user manually intervened. Instead, cbmk always runs the crossgcc build script from coreboot; coreboot itself has a much more thorough set of checks, skipping a given build that was already performed, but the check is a bit slower, adding a few additional seconds to the build time per target. The bug that this fixes was in fact triggered, during random testing, leading to the fix.
• include/lib.sh: Fixed error when running ./build dependencies. The e() and setvars() functions were declared after the dependencies function, but are now declared before. Also: after calling install_packages, it was doing a return when it should have done an exit. All of this lead to certain temporary files being created; the context here was that the user would be running ./build dependencies as root, which lead to some of those files being created as root, but cbmk exits with not-zero status when you run it as root, except when running the dependencies command. Therefore, this could create situations where the user can longer run cbmk without intervention such as changing permission on certain files. Avoid the issue entirely.

General code cleanup

Extensive code cleanup has been performed, as was the main purpose of Audit 2.

The changes are as follows:

• Removed util/autoport, because the Haswell support that we merged for it is now included in upstream(coreboot), so it will be included in future coreboot revisions for cbmk.
• script/trees: Move dependency building (based on build_depend) to a new function, called from configure_project(), instead of including the logic directly within the latter.
• script/trees: Merge build_targets() into handle_targets().
• script/trees: Use : as a prefix on certain build commands, when doing try runs (dry builds); the -d flag was initially implemented with a lot of if/else chains, but : does nothing in sh (that's what it's for, to do nothing), including if arguments are provided to it. This change enables much more granually disablement of certain commands, which would otherwise require many more if/else chains. This change reduced the amount of code.
• script/trees: Remove all project-specific hacks. The coreboot-specific includes were moved to coreboot's mkhelper.cfg file. Serprog and coreboot images are handled by mkhelper functions now.
• include/rom.sh: It also provides building of serprog images. The mkhelper configuration is provided for pico-serprog and stm32-vserprog. The old ./build roms serprog commands still work, but you are now advised to run ./update trees -b stm32-vserprog or ./update trees -b pico-serprog, which will now result in the same builds being provided under bin/.
• include/rom.sh: New file, replacing script/roms. It contains the same functions, more or less, for building coreboot images. The trees script already produces coreboot images, and payloads were added by script/roms; now, those same functions (for adding payloads) are stubbed via mkhelper configuration in config/data/coreboot/mkhelper.cfg. The command ./update trees -b coreboot (whether specifying additional targets, or specifying none and thus building all of them), now builds coreboot images with payloads inserted, and never leaves them on-disk without payloads; this removes an existing issue where the user might accidentally use the no-payload images, mitigated by provided a warning but still an issue regardless (for example, what if the user doesn't understand English very well?). The old ./build roms command is now deprecated, but still provided for backward compatibility, but you are now advised to run the trees command. The bin/ directory is still the place where coreboot images go, but they no longer appear(without payloads) under elf/ - only the bin/ images are provided.
• script/roms: Removed unnecessary calls to ./update trees -b for payloads, because coreboot targets now specify build_depend which is used generically per tree, per multi-tree project, to provide such build dependencies.
• script/trees: Rename function load_target_config() to configure_project(), because the function also now handles building to some extent (based on build_depend), not just mere loading of config files.
• include/git.sh: Simplified git_am_patches() by condensing several for loops together, into a single for loop performing each task.
• include/git.sh and script/trees: Tidy up the use of global variables. Some of them were only initialised in git.sh but also used in the trees script, which means they should be initialised in the trees script.
• include/git.sh: Simplified initialisation of loc (single code line)
• script/trees: Simplified distclean directory check, by reducing the nesting of if/else statements.
• include/git.sh: Condensed a few code lines in fetch_targets().
• include/git.sh: Shorter git_prep command in fetch_targets().
• script/trees: Simplified multi-tree bare repository cloning. The git_prep function already creates a given directory where source code goes, so we don't need to handle it from the trees script.
• script/trees: Merged prepare_new_tree() with fetch_targets().
• script/trees: Simplified distclean handling; condensed a few code lines.
• script/trees: Further simplified copy_elf(); condensed a few code lines.
• script/trees: Added explicit return to the end of handle_defconfig().
• script/trees: Renamed check_config() to check_defconfig(), for clarity.
• script/trees: Removed variable config_name, because it's only used once, so its value was instead hardcoded, without declaring a variable.
• script/trees: Renamed variable config to defconfig, for clarity.
• include/git.sh: Removed redundant xtree variable, already defined in trees.
• script/trees: Removed various comments that were redundant, because the code that they covered was already simple enough that what they did was obvious.
• script/trees: Cleaned up handle_makefile() a bit; condensed a few lines.
• script/trees: Renamed load_project_config() to load_target_config(), for clarity.
• script/trees: Download multi-tree bare repositories before given trees. When downloading multi-tree projects, revisions can be reset to HEAD instead of the actual revision for a given target. This occurs when the bare repository (e.g. src/coreboot/coreboot) does not exist and has to be fetched first. Bare repository downloading does not rely on target.cfg, only pkg.cfg, but uses the same variables, e.g. rev. So: instead of using a separate variable name, do the bare repository download first. This means that the git.sh logic can be much cleaner, in that it only copies the bare repository and then runs git_prep. The bare repository is closed directly by calling the relevant function from trees, which is therefore the same behaviour as when cloning single-tree projects.
• script/trees: The _setcfgarg variable was removed, in function load_project_config() because it became disused and thus redundant.
• script/trees: Unified multi-tree and single-tree configuration handling. The same functions that load configurations for single- and multi-tree have been merged with the one in git.sh, and then that was moved back into the trees script. Now git.sh only handles the actual downloading and patching of sources, but not configuration of them; the latter is handled directly within the trees script. The benefit of this change is that it generally allows far more flexibility; many functions that use such configuration logic can now be unified under this script, for instance using the mkhelper feature mentioned elsewhere on this page - and that is precisely what was later done, in further changes that are detailed above.
• script/trees: Unified handling of source downloads. Run it from function load_project_config(). The logic used to be split, between single- and multi-tree projects, but now it is consistent throughout.
• include/git.sh: Rename fetch_project_trees() to fetch_targets().
• include/git.sh: Rename fetch_project_repo() to fetch_project().
• script/trees: Add explicit return at the end of check_coreboot_utils().
• Generally tidied up a few setvars calls, condensing them them so that they used fewer code lines, while still observing the 80-character rule.
• script/roms: Explicitly shift by 1 for all targets. The way this script worked, it would go through all arguments populating each one inside a single string containing them, shifting until no arguments were left. We want to make sure it is shiftedh by one, and make this intention clear.
• script/roms: Add return value for the list command, because we want a specific return value but return passes the value of the previous return, if a given return call does not specify an argument, and there's no way to predict what the previous value would have been. However, this is not a bug fix, pre-emptive or otherwsise, because nothing actually using this command relied in any way on its return value.
• include/lib.sh: Tidy up err_(); condense a few code lines.
• include/lib.sh: Add explicit return to the end of chkvars().
• script/trees: Simplify single- and multi-tree handling in main(). Use shorthand notation on a single line, initialising it based on single-tree and then checking that the given project is multi-tree.
• script/trees: Rename build_projects() to build_project(). The script used to support building multiple single-tree projects, but this behaviour was buggy and unused, so it was removed. Rename the function accordingly.
• script/trees: Set btype if target.cfg doesn't exist; set to auto. The build system assumes multi-tree, if Kconfig files are used. Single-tree projects will not typically define btype, so just default it to auto.
• include/lib.sh: Removed unused cbdir variable.
• script/roms: Don't assign cbdir, because it wasn't even used here.
• script/trees: Don't hardcode skipping kconfig files based on whether the project is GRUB. Instead, define btype in project target.cfg files, and define this in GRUB. Some projects are multi-tree but do not use Kconfig files, and GRUB is one of them; we must avoid commands such as make menuconfig, doing nothing and returning with zero status explicitly, otherwise it would yield an error, which is a problem if you ran the trees script with a flag but no argument (which, as documented elsewhere in this report, was later made to operate that flag on every project in succession).
• include/git.sh: Don't check that a given source directory exists, because the main trees script already does this before running fetch commands, which are implemented in git.sh.
• build: Move git_err() here, from include/lib.sh, because it's only used in the main build script.
• include/lib.sh: Simplify singletree(); condense a few code lines.
• include/lib.sh: Add an explicit return to the end of check_defconfig().
• include/lib.sh: condense the e() function a bit (was already done before, and done again).
• include/lib.sh: Simplified TMPDIR handling; remove the tmpdir variable and use TMPDIR directly. Use a new variable xbmk_parent, which is set to y only on the main instance; child instances of cbmk do not set it, signalling that TMPDIR should not be removed upon exit (it should only be removed upon exit from the main parent instance of cbmk, not child instances executed by it).
• include/lib.sh: Condense if/else logic in setcfg(). Use shorthand notation instead, on conditional statements.
• include/lib.sh: Condensed setvars() a bit.
• include/lib.sh: Simplified lock message, when a lock file exists while trying to run cbmk.
• include/lib.sh: Simplified reading of version files. A single for loop now initialises all of these variables with a single eval command inside.
• include/git.sh, include/lib.sh, script/roms and script/trees: Simplify use of environmental variables. The variables themselves were being copied to other variables, of lowercase naming, but this was unnecessary. The environmental variables are now used, directly, while being properly set per each running instance of cbmk.
• script/roms: Remove the t variable, which was used for temporary ROM images. The script now operates on images more directly, and this variable is no longer used (previously used for storing paths to temporary images).
• script/roms: Simplified logic for checking U-Boot builds.
• script/roms: Simplify initmode/displaymode loop, when building multiple coreboot images per target.
• script/roms: Renamed a few functions and variables for code clarity.
• script/roms: Build coreboot images earlier on in the process, to avoid duplicate work. It was built per coreboot configuration, but the logic builds all images per target, so it doesn't make sense to run the build twice. The subsequent runs would just skip, because the images already existed.
• script/roms: Generalised more logic, merging a lot of functions and reducing a lot of code repetition.
• include/lib.sh: Remove badcmd(), because it's bloat. This function simply write "bad command", followed by a call to the err function. Instead, simply call err with the same string, when required.
• include/lib.sh, include/git.sh, script/trees and script/roms: More unified handling of project configurations. Remove the function scan_config(), which worked with a specially crafted format that we had for all of the project (source repo) configs. Instead, use flat config files. This change resulted in a sloccount reduction of about 20, in the build system, and the new config handling is much more robust.
• script/trees: More robust check for make fetch, when handling source downloads. Don't use shorthand form; instead, use if/else chains to determine whether to skip a target after download.
• include/git.sh: General code cleanup in fetch_submodules(). Several code lines have been condensed.
• script/roms: Merge mkserprog() into main(), because it was essentially performing the same functions, but for checking serprog targets. Unify the logic, for handling coreboot, stm32 serprog and rp2040 serprog targets.
• script/roms: Reduced code indentation in build_grub_roms().
• script/roms: Remove build_payloads() and split it into smaller functions.
• script/roms: Condense lines together that check for various payloads.
• script/roms: Removed the mt86bin variable, because it's only used once. Instead, hardcode the path to memtest86plus when building it.
• script/roms: Merge build_uboot_payload() into build_payloads(), because both functions are very simple and do essentially the same thing.
• script/roms: Simplified check for u-boot payload; we only need to know whether it's enabled, not whether it's also disabled. If set to enable other than y, set it to n blindly.
• script/roms: Simplify the check for serprog source directories. Instead of an if/else chain, re-use variables and use a single eval command between the two types; serprog rp2040 and serprog stm32.
• script/roms: Simplify the loop for building serprog images, by re-using variables more aggressively and removing the if/else chain for serprog type.
• script/roms: Shorter variable names for serprog build logic.
• script/roms: Simplified serprog image copy, by using a single eval call referencing stm32src or rp2040src, thus unifying this logic between both types of serprog build.
• script/roms: Rename picosrc to rp2040src; this is in preparation for the next change, mentioned above.
• script/roms: Remove confirmation dialog at the end of a serprog build, because the message was overly pedantic and something the user already knows.
• script/roms: Merge the serprog handling into a single helper function.
• script/roms: Renamed the x variable to it in a for loop, to avoid possible conflict with another same-named variable elsewhere in this script.
• script/roms: Don't call x_ when using cproms(), since it never returns with non-zero status anyway.
• script/roms: Move the U-Boot images, instead of copying. This way, we don't have to clean up the temporary file that was used for it.
• script/roms: Allow flexible use of mv or cp in cproms().
• script/roms, include/lib.sh and script/trees: Use a common string for setting elfdir, which is where builds are copied to upon completion.
• script/roms: Merge mkUbootRom() into build_uboot_roms().
• script/roms: Remove unused variables in mkSeabiosRom().
• script/roms: Remove unused variables in mkUbootRom().
• script/roms: Remove unnecessary variable assignment; cbcfg was already properly initialised as a global variable, so this was re-used instead.
• script/roms: Removed unnecessary check for payload_uboot; the uboot_config variable, if set, clearly indicates use of U-Boot.
• build: Remove unused linkpath and linkname variables.
• script/roms: Make tmpcfg a variable, re-used elsewhere when a temporary file needs to be written. Simply overwrite the file as required, instead of always calling mktemp.
• script/roms: Simplified timeout/scandisk insertion. We don't need to call mktemp every time. Just use a static temporary file and keep overwriting it, when we need to make a new configuration file inside CBFS.
• script/roms: Simplified SeaGRUB check in build_grub_roms(); condensed a few code lines.
• script/roms: Simplify mkserprog(); condensed several code lines and removed unused variables!
• script/roms: Simplify the Serprog build functions; condensed several code lines and removed unused variables.
• script/roms: Simplify U-Boot payload check (same change as below).
• script/roms: Simplify GRUB-only check on SeaGRUB (same change as below).
• script/roms: Simplify SeaGRUB check in configure_target(); condensed the if/else checks based on which payloads are configured.
• script/roms: don't use x_() to call build_grub_roms(), since that function never returns non-zero status anyway.
• script/trees: Simplify copy_elf(); condense the while loop handling file copies, for copying builds to their destination paths once complete.
• script/trees: Remove unnecessary check in copy_elf(); we don't need to check whether the listfile string is empty, because a check comes after it as to whether the file it defines exists, and checking an empty string there will always yield a negative result, as intended.
• include/lib.sh: Add generic cbfs() function, which is now used throughout cbmk when adding files to CBFS on coreboot images. This simplifies handling of CBFS in cbmk.
• script/roms: merged handling of build targets into the main script, simplifying the loop going through them all; each helper function returns with non-zero status if the build is to cease, and the target is skipped.
• script/roms: Simplified handling of multiple coreboot build targets, when looping through them all during the build process.
• include/lib.sh: added a unified function chkvars(), now used for checking variable initialisation; if a variable is not initialised, it causes an exit with non-zero status.
• script/roms: shorter variable names, condensed several functions.

Revision updates

U-Boot

Alper Nebi Yasak is the maintainer of U-Boot, within Libreboot, and a Libreboot patch of his updated U-Boot to v2024.07, on the gru_bob and gru_kevin Chromebooks. Several improvements have been merged upstream, like Alper's patches for Qemu, so these patches have been dropped in cbmk because they're now included by default, in the new U-Boot revision. This patch has been imported into Canoeboot, from the Libreboot project.

Alper also tested the latest coreboot revision and it worked fine, on these boards, but revision updates will be done immediately after Audit 2 is announced, as a priority for further work that is to be completed for the next Canoeboot release, ETA August 2024.

Git log

This git log covers all changes in this audit, relative to Canoeboot 20240612.

* 16f9ad55ca put cachedir in environmental variable 
* 6b2653f8cd git.sh: warn when a cached clone fails 
* 1458e1108e git.sh: fix typo in git command 
* 20b076714c git.sh: fix lack of error exits on fault 
* a7f9a7bb6f build: remove tmp/ on release archives 
* 71091e03a7 trees: hardcode makeargs for xgcc 
* 1bc484dbf6 allow using coreboot's build system to add payload 
* 93a476b4fc trees: remove unnecessary command 
* ae3966a02f build: remove cache/ on making releases 
* 0cf0fdcfc3 unify caching of files into cache/ 
* 8bb3730e7b cache downloaded files(module) to cache/file/HASH 
* 0d55d7b23a git.sh: remove previous tmprepo if args>5 
* 3e2e5ecf5a git.sh: try direct clone if cached git fails 
* 03c9670fef git.sh: re-try git pull three times 
* 055c9be15b trees: auto-delete+auto-rebuild if project changes 
* 2d794a8385 trees: also remove single-tree repo/p/ 
* b564f5b848 trees: remove repo/p, not src/p/p 
* c44ff5ac47 git.sh: don't download to src/project/project/ 
* 3590a53ed1 git.sh: cache git downloads to repo/ 
* 2823ccc438 remove util/autoport 
* a29453023c trees: move dependency building to new function 
* a9997082d7 u-boot: Fix display initialization on gru boards 
* 5b170de055 u-boot: Avoid breaking build for U-Boot-only binman images 
* d442d61f84 u-boot: Update to v2024.07 
* 215764cfbd trees: fix bad comparison in configure_project() 
* 60d491e2bd trees: don't continue if no argument given 
* 3365fca06a trees: general code cleanup 
* a721e927b0 trees: merge build_targets() with handle_targets() 
* 1a95c0cf30 trees: use wrapper for dry-running make commands 
* a258eb231a trees: remove project-specific hacks 
* 3681c29e77 remove executable permission on include/ 
* 378c09a25c rom.sh: actually say if a cbutil exists 
* 87681db12c rom.sh: avoid re-building cbutils if built 
* fdf7864905 rom.sh: only before cbutils before coreboot 
* 21306dd5b8 trees: fix bad comparison 
* 9a3beea79c minor cleanup 
* 4dbce8aef0 trees: support -d (dry run) for custom build logic 
* 2dad7b0b6f rom.sh: only make rom tarball if release=y 
* e01995d491 rom.sh: new file, to replace script/roms 
* a50c789b1d roms: remove unnecessary trees checks 
* e5262da7ca coreboot: set build_depend on target.cfg files 
* a8a42ebb0b trees: rename load_target_config() 
* 07769f3341 trees: support build dependencies, not just fetch 
* 1b75d738bf GRUB: only load xhci from grub.cfg 
* bfeab80a8d trees: just do makeargs on coreboot, not cbmakearg 
* 05b59f39d6 trees: fix bad rm -Rf command (-Rf, not Rf) 
* 5d179fe3e0 roms: fix bad comparison in configure_target() 
* 1fe126501a GRUB: use mkhelper.cfg for common variables 
* c76e6b0527 trees: allow global config on multi-tree projects 
* bdf43e07cf trees: handle all projects if project undefined 
* ff00073666 git.sh: simpler for loop in git_am_patches() 
* de26bb9997 git.sh: merge for loops in git_am_patches() 
* cc090de51e trees and git.sh: tidy up global variables 
* 5b24c812a0 git.sh: simplified initialisation of "loc" 
* a62ad20730 trees: simplified distclean directory check 
* 63ae4ad746 git.sh: condense fetch_targets() a bit 
* 7aad37199c git.sh: short git_prep command in fetch_targets() 
* 4a152d53c3 trees: only do bare multi-tree clone on git 
* be4c655008 trees: simplified multi-tree bare repo clone 
* 51fe371931 git.sh: merge prepare_new_tree with fetch_targets 
* ca77d3a3d7 trees run_make_command: simpler distclean handling 
* aad9c8d0de trees: condense copy_elf() a bit 
* fdd67e5935 trees: add return to handle_defconfig() 
* be01e56609 trees: rename check_config to check_defconfig 
* 2740db84b7 trees: remove variable "config_name" 
* c77c09bac2 trees: rename variable "config" to defconfig 
* 675b24e83d git.sh: remove duplicate "xtree" variable 
* e332937241 trees: remove unnecessary commonts 
* 516b399eb1 trees: condense run_make_command() a bit 
* 0ab9afc7af trees: condense handle_makefile() a bit 
* 605b099e04 trees: mv load_project_config load_target_config 
* ef3ff3dfe4 trees, multi: download bare project *before* trees 
* e377404406 trees: unified multi-tree configuration handling 
* 211f4f0b82 trees: unified handling of source downloads 
* 0f7a5c27dc git.sh: rename Fetch_project_trees fetch_targets 
* 626fd9f245 git.sh: rename fetch_project_repo to fetch_project 
* 6a91dc02dc trees: better skip-clean string in handle_src_tree 
* de8dc508f9 trees: add return to check_coreboot_utils() 
* cc61bd1239 trees: simplify "utilmode" variable initialisation 
* cbe40044b4 trees mkpayload_grub: remove unnecessary check 
* 7322a2b53d lib.sh: stricter check in chkvars() 
* 203fdb8007 tidy up some setvars lists 
* d3ccb4d8a7 roms: explicitly shift by 1 on the "all" target 
* bd8f0e0bbb roms: add return value for the list command 
* 5907022703 roms: build u-boot *before* checking ubootelf 
* 57b68302c7 remove more unnecessary checks on trees commands 
* 0dcd8852dd lib.sh: keep versiondate check to 80 characters 
* 2ebdd184b7 lib.sh: condense for loop 
* 5dc30167de lib.sh: condense err_() a bit 
* a9882cfa00 lib.sh: add a return to the end of chkvars() 
* 511423a85a lib.sh: remove unused variable "boarddir" 
* 1fbfc7303d remove use of _xm variable in the build system 
* 7451fa629c trees: don't hardcode use of mkpayload_grub 
* 623ad90ef0 trees: simplify single/multi handling in main() 
* 7b178f5fad trees: rename build_projects to build_project 
* 7ee147ed49 trees: err if target.cfg not given if multi-tree 
* f8d1abf18d trees: set btype if target.cfg doesn't exist 
* 2827917b0a lib.sh: remove unused cbdir variable 
* dd28339f38 roms: remove unnecessary assignment to cbdir 
* cf4f828dbe trees: avoid kconfig make commands generically 
* ce9b2f0a1c git.sh: remove unnecessary check 
* f3baebe7e1 lib.sh: move git_err() to build 
* 43238fa0c5 lib.sh: condense singletree() a bit 
* de331e5da0 lib.sh: add a return to the end of check_defconfig 
* 94d9d313cc trees: condense elfcheck() a bit 
* 35c516c2f4 lib.sh: condense e() a bit 
* 2ac6db2b91 trees: shorten the final confirmation message 
* 9ba28a0b86 lib.sh: make elf/coreboot* a dot directory 
* 32e1b13416 build: don't rm TMPDIR if it's /tmp 
* 6daea94df8 lib.sh: simplified TMPDIR handling 
* 0c0b8124c1 lib.sh: condense setcfg() if/else logic 
* 0a2ed1a7bd trees: remove redundant space in printf 
* e3179f435e trees: explicitly err if OPTARG is not set 
* 8dcfb8a585 trees: only permit one single-tree project 
* c56531af40 trees: call err if multiple flags are provided 
* ebebb5a57e trees: explicitly set mode for -b 
* 0662b22297 roms: re-add compression of coreboot images 
* e8b7e74db5 roms: build coreboot *after* checking variables 
* aac8720382 lib.sh: introduce mandatory check of projectname 
* ae28debf21 lib.sh: condense setvars() a bit 
* 8c06c62e06 simplified lock message 
* 2965d526fd lib.sh: simplify reading of version files 
* 3319147306 lib.sh: simplify use of environment variables 
* aa89f69b34 roms main(): confirm what serprog images are built 
* ba228ac228 roms: remove unused variable name 
* e48183a52e roms: remove redundant printf 
* e73f1b1c9b roms: optimise u-boot elf check 
* 14c3a328f1 roms: simplify build_roms() 
* 0355a80408 roms: make the bin/elf message more polite 
* 537efc148d roms: re-add final confirmation of targets 
* 01986beda3 roms: rename functions for extra clarity 
* d0a949d00b roms: build coreboot early to avoid duplicate work 
* df66dbd72d trees: try xgcc build twice if first attempt fails 
* bd59d01ea2 trees: don't check if xgcc is already built 
* fc408f5554 lib.sh: fix error running ./build dependencies 
* 8a02aef1d8 remove unused git modules 
* b72dfe11f9 roms: general code cleanup 
* 852eb1db4f roms: only support SeaBIOS/SeaGRUB on x86 
* 107072b189 roms: remove support for "grubonly" seabios 
* ad1d0cb58c use backticks on eval commands, not subshells 
* e7fcfac14e lib.sh: remove badcmd() 
* dec9ae9b43 lib.sh: more unified config handling 
* c72904b6d1 trees: more robust check to avoid "make fetch" 
* a59ebb1b7c roms: fix lack of backslash on multi-line command 
* 253015f6a9 Revert "roms: remove build_payloads() and split it up" 
* 448d02babb git.sh: revert modification to for loop 
* 381ed442d9 minor code cleanup in the build system 
* 295471644a git.sh: general code cleanup in fetch_submodule() 
* 3ba876932d git.sh: reduced indentation on repo/file check 
* 109db65932 git.sh: simplified repo/backup check 
* acbefc558e roms: merge mkserprog() into main() 
* c2ca92a169 roms: don't insert timeout.cfg 
* 98fc84b0ed correction 
* 3ef9925b56 roms: reduce indentation in build_grub_roms() 
* faddd02ef5 roms: re-introduce accidentally disabled check 
* 14a05f72fb roms: remove build_payloads() and split it up 
* da3d7c7101 roms: group some commands that are similar 
* 99a13a32d9 roms: remove mt86bin variable 
* 3097a9e97a roms: merge build_uboot_payload to build_payloads 
* ccb330ec09 roms: simplify payload_uboot y/n check 
* 49c41411bd roms: simplify the check for serprog srcdir 
* 8136b640b9 roms: simplify the loop for building serprog roms 
* 6976a4edb2 roms: shorten variable serprog_boards_dir 
* 4a3ab3ad1c roms: simplified serprog image copy 
* 64ce81d2b6 roms: rename picosrc variable to rp2040src 
* e3471b22e7 roms: remove useless confirmation in mkserprogfw 
* a8fb99a750 roms: merge serprog build into one function 
* b1ec5ad719 roms: remind the user about gkb files 
* a93f519799 roms: rename x variable to it in for loop 
* faff6f0348 roms: don't use x_ to call cproms() 
* f43525d14f roms build_uboot_roms(): move rom, don't copy 
* dd2854b3e8 roms cproms(): allow other commands besides cp 
* a8f44ab80a unify coreboot elfdir (DO_NOT_FLASH) 
* be04f59602 roms: merge mkUbootRom() into build_uboot_roms() 
* a9819d7385 roms: simplify mkSeabiosRom() 
* 19baf8d1a7 roms: simplify mkUbootRom() 
* 935958e6a4 roms: simplify build_roms() 
* acd5d16961 roms: remove unnecessary check 
* 6094542663 build: remove unused variables 
* 94b9bd44e4 roms: further clean up build_grub_roms() 
* 167f81a993 roms: simplify timeout/scandisk insertion 
* d958b16369 roms: simplify seagrub check in build_grub_roms 
* 726b56b2f1 roms: simplify mkserprog() 
* dd59f2daba roms: simplify the serprog build functions 
* 9897bc748d script/roms: fix serprog build commands 
* 3269e0c097 roms: simplified ubootelf check 
* 7265b9f112 roms: simplify grubonly check in configure_target 
* c29b3ec3fc roms: simplify seagrub check in configure_target 
* f07aa84d49 roms: don't use x_ to call build_grub_roms 
* ed921d7890 trees: simplify copy_elf() 
* 293777fb99 trees: remove unnecessary check in copy_elf 
* b50a588cba grub: insert background in memdisk instead 
* 7f5adffc02 roms: unify all add-payload commands 
* 82c6a352df roms: don't add grub keymaps at all 
* f1aea87141 roms: merge handle_coreboot_target into main() 
* ae9dce889b roms: simplify target check (whether roms built) 
* bb4f0995d1 roms: simplify main() again 
* 59392ea0bb roms: remove redundant check on grub_scan_disk 
* e96d85dc76 roms: remove dangerous runtime p/s/d options 
* bb7b04cb59 unified checks for variable initialisation 
* 1399f2137f lib.sh: remove the items() function 
* 4ed6e41221 roms: simplify main() 

This is 202 changes in total, between 12 June 2024 and 19 July 2024.