fit_image_get_data() checks that the external-data region fits in the addressable range and, for signed FITs, stays below FIT_SIGNATURE_MAX_SIZE. The SPL loader had no equivalent check, so a hostile data-offset or data-size could make the read offset wrap past ULONG_MAX, or the destination range wrap around the end of the address space.
Add the same check to load_simple_fit(), using the block-aligned read size and the source pointer the read actually uses. Signed-off-by: Anton Ivanov <[email protected]> --- Code size impact of this patch alone (buildman -S, gcc 13.3), measured on arm and aarch64: 387 of the 413 boards that build spl_fit.c. All of it is in SPL; U-Boot proper is unchanged: spl/u-boot-spl: text +24..+64, rodata +57 (all +81..+121) All of the text growth is in load_simple_fit(); the +57 rodata is exactly the new printf() string - happy to change it to log_debug() if that is preferred for SPL. --- common/spl/spl_fit.c | 26 ++++++++++++++++++++++++++ test/image/spl_load.c | 23 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 9b0bee25b8b..ec66707efe7 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -343,6 +343,32 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len); length = len; + /* + * For non-signed FIT images, we can check that + * (read_offset + size) does not wrap and that + * (src_ptr + size) does not exceed the addressable range. + * For signed FITs, we can additionally check that + * (offset + len) doesn't exceed the allowed FIT image + * maximum size. + */ + if (size > ULONG_MAX - read_offset || + size > UINTPTR_MAX - (uintptr_t)src_ptr + /* + * #if (not a runtime if) is required: FIT_SIGNATURE_MAX_SIZE + * depends on FIT_SIGNATURE, so CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) + * is undefined when signing is disabled and referencing it + * here would fail to compile. + */ +#if CONFIG_IS_ENABLED(FIT_SIGNATURE) + || offset > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) || + len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset +#endif + ) { + printf("FIT external data is out of bounds (offset=%u, size=%u)\n", + offset, len); + return -EINVAL; + } + log_debug("reading from offset %x / %lx size %lx to %p: ", offset, read_offset, size, src_ptr); diff --git a/test/image/spl_load.c b/test/image/spl_load.c index 557362ba113..334ec77b18e 100644 --- a/test/image/spl_load.c +++ b/test/image/spl_load.c @@ -483,6 +483,29 @@ static int spl_test_fit_read_offset_overflow(struct unit_test_state *uts) } SPL_TEST(spl_test_fit_read_offset_overflow, 0); +/* + * A read whose end position (read_offset + size) wraps past the addressable + * range must be rejected. + */ +static int spl_test_fit_read_end_overflow(struct unit_test_state *uts) +{ + if (!image_supported(FIT_EXTERNAL)) + return -EAGAIN; + + /* + * The aligned external-data offset (0x2000 plus the size of the FIT + * itself) stays below the 0x2fff bytes remaining before ULONG_MAX, so + * read_offset passes the offset-wrap check above, but reading the + * SPL_TEST_DATA_SIZE bytes of data crosses past UINTPTR_MAX. + */ + spl_test_fit_offset = ULONG_MAX - 0x2fff; + + return check_fit_ext_prop(uts, FIT_DATA_OFFSET_PROP, 0x2000, 1, + spl_test_read_fit_offset, spl_test_fit_offset, + -EINVAL); +} +SPL_TEST(spl_test_fit_read_end_overflow, 0); + /* * LZMA is too complex to generate on the fly, so let's use some data I put in * the oven^H^H^H^H compressed earlier -- 2.55.0
