Link to v1: https://lore.kernel.org/u-boot/[email protected]/
Changes from v1 (all from Simon's review of patch 1): - Move the shared error label to the end of the function so no goto jumps into an if-body. Both bound checks now goto too_big and the printf/return sits after the final return 0. - Rewrite the comment on the early len > max_size check. The real reason it exists is to keep get_aligned_image_size() from running its int arithmetic on a hostile value; the block-aligned size check further down is the mathematically binding one because size is len rounded up. - Include the data-size value in the diagnostic printf so a real failure is triageable from a board log. - Note in the fix commit message that CONFIG_SYS_BOOTM_LEN is a conservative ceiling for the firmware/loadables/FDT/FPGA callers rather than the exact capacity at the destination. The overlay caller remains the one that is actually tight against its buffer. --- This series closes a pre-authentication memory corruption window in the SPL FIT loader. load_simple_fit() in common/spl/spl_fit.c reads external image data from the boot device with a transfer sized from the FIT data-size property, and only verifies the image hash after the read has already happened. data-size is in the exc_prop[] exclusion list in boot/image-fit-sig.c, so it is not covered by the configuration signature and stays under the control of anyone able to modify the boot medium even when CONFIG_SPL_FIT_SIGNATURE is on. An inflated value drives info->read() into the destination before fit_image_verify_with_data() can reject the image. The device-tree overlay path is the sharpest case, because there the destination is a fixed CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY_BUF_SZ heap buffer rather than a sized load region. Patch 1 adds a max_size argument to load_simple_fit() and rejects both the raw data-size and the block-aligned transfer length with -EFBIG before the device read is issued. The size check is the mathematically binding one because size is len rounded up to the device block length; the early len check exists so get_aligned_image_size() never runs on a hostile value, where its int arithmetic would invoke signed-integer overflow. Callers that load into a memory region pass CONFIG_SYS_BOOTM_LEN, the same limit spl_parse_legacy_validate() already applies to legacy images. The overlay path passes the size of its temporary buffer. Patch 2 adds a sandbox regression test in test/image/spl_load.c. It builds a FIT with external data, then rewrites the data-size property in place to a value far beyond both the image buffer and any plausible load region, and asserts that spl_load_simple_fit() returns -EFBIG instead of overrunning the destination. Without the fix the test crashes inside the test-harness read callback when memcpy() walks past the small calloc'd image; with the fix the load is rejected cleanly. Verified on sandbox_spl_defconfig: the new test passes, and the full SPL unit test suite reports 20 run, 1 skipped, 0 failures. Also cross-built am335x_hs_evm_defconfig and am335x_hs_evm_spi_defconfig with arm-linux-gnueabihf-, both within the SRAM budget. On the size question from v1 review: the cleaner shape (single too_big label at end of function) plus the expanded printf now costs 71 bytes text on am335x_hs_evm (vs the 57 bytes v1 quoted for the goto-into-if shape). text+data lands at 45132 out of the CONFIG_SPL_MAX_SIZE = 0xb0b0 = 45232 budget, so 100 bytes of headroom. Aristo Chen (2): spl: fit: bound the external data size before reading it test: spl: check load_simple_fit() rejects an oversized data-size common/spl/spl_fit.c | 51 ++++++++++++++++++++++++++++++++++++++----- test/image/spl_load.c | 49 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 6 deletions(-) -- 2.43.0
