Hi Marcel, Your commit reverted two patches related to the hardening of FIT external data processing for SPL: [PATCH v4 3/4] spl: fit: Harden external-data offset and size arithmetic [PATCH v4 4/4] spl: fit: Bound external data like U-Boot proper
For this board, the binman FIT uses embedded data only, so the hardened external-data code is never reached in `load_simple_fit()`. The only path that is changed and runs from `spl_simple_fit_read()` is `get_aligned_image_size()`, and with `bl_len=1` (SPL RAM boot), it cannot fail. I tried to reproduce the issue on the emulated hardware, but I couldn't make it hang as you described. I suspect there is a layout issue with the hardware itself, so I would perform a debugging using the device as the next step. Thank you, Anton On Sat, Oct 10, 2026 at 8:55 PM Marcel Ziswiler <[email protected]> wrote: > Hi Anton, et. al. > > On Wed, 2026-09-02 at 22:23 +0100, Anton Ivanov wrote: > > The data-offset, data-position and data-size FIT properties are > > excluded from the configuration signature, so they are attacker > > controlled even when signature verification succeeds. The offset and > > size arithmetic in load_simple_fit() can wrap on hostile values: > > > > - adding the external-data base offset to data-offset can wrap past > > UINT32_MAX, > > - get_aligned_image_size() adds the block-alignment overhead and > > rounds up to the block length, which can wrap past ULONG_MAX, > > - adding the FIT's device offset to the aligned external-data offset > > can wrap past ULONG_MAX. > > > > Make get_aligned_image_size() return the aligned size through an out > > parameter and fail with -EOVERFLOW when the computation would wrap, > > check the two offset additions explicitly, and compare the > > block-aligned size (the amount info->read() actually transfers) > > against max_size before reading. Do the same for the FIT header read > > in spl_simple_fit_read(). > > I bisected this commit to regress on the LicheePi 4A in the sense that the > SPL is no longer able to > load/execute OpenSBI/U-Boot proper: > > U-Boot SPL 2026.10 (Oct 10 2026 - 21:45:55 +0200) > Trying to boot from RAM > => hangs > > My fix reverting this and a second patch [1] got accepted in meta-riscv, > see: > > https://github.com/riscv/meta-riscv/pull/711 > > Before I dig deeper, I would welcome any insight anybody could have. > Thanks! > > Cheers > > Marcel > > [1] https://lore.kernel.org/all/[email protected] > > > Signed-off-by: Anton Ivanov <[email protected]> > > Reviewed-by: Simon Glass <[email protected]> > > > > --- > > Changed since v3: the new diagnostics use log_debug() instead of > > printf(), which compiles out in a non-debug SPL build. > > > > Code size, from buildman -S in the u-boot-gitlab-ci-runner container > > (gcc 14.2.0). The 413 boards that build spl_fit.c cover seven > > combinations of CONFIG_SPL_LOAD_BLOCK (BLK), CONFIG_SPL_FIT_SIGNATURE > > (SIG) and bitness, one board each below. SPL only; U-Boot proper is > > unchanged: > > > > BLK SIG bit board text rodata data all > > y n 32 am335x_evm +24 -49 -4 -29 > > y y 32 firefly-rk3288 +20 -49 0 -29 > > n n 32 am335x_evm_spiboot +4 -49 0 -45 > > n y 32 evb-ast2600 -8 -49 0 -57 > > y n 64 imx8mm_evk +16 -49 0 -33 > > y y 64 orangepi-5-ultra-rk3588 +28 -49 0 -21 > > n n 64 r8a779g0_whitehawk 0 -49 0 -49 > > > > The rodata drop is the "FIT image too large" printf() this patch > > removes. > > --- > > common/spl/spl_fit.c | 98 > +++++++++++++++++++++++++++++---------------------- > > test/image/spl_load.c | 57 ++++++++++++++++++++++++++++++ > > 2 files changed, 112 insertions(+), 43 deletions(-) > > > > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c > > index 9b16f91cc6f..65934674b6c 100644 > > --- a/common/spl/spl_fit.c > > +++ b/common/spl/spl_fit.c > > @@ -175,22 +175,32 @@ static int spl_fit_get_image_node(const struct > spl_fit_info *ctx, > > return node; > > } > > > > -static int get_aligned_image_offset(struct spl_load_info *info, int > offset) > > +static u32 get_aligned_image_offset(struct spl_load_info *info, u32 > offset) > > { > > return ALIGN_DOWN(offset, spl_get_bl_len(info)); > > } > > > > -static int get_aligned_image_overhead(struct spl_load_info *info, int > offset) > > +static u32 get_aligned_image_overhead(struct spl_load_info *info, u32 > offset) > > { > > return offset & (spl_get_bl_len(info) - 1); > > } > > > > -static int get_aligned_image_size(struct spl_load_info *info, int > data_size, > > - int offset) > > +static int get_aligned_image_size(struct spl_load_info *info, ulong > data_size, > > + u32 offset, ulong *aligned_size) > > { > > - data_size = data_size + get_aligned_image_overhead(info, offset); > > + u32 overhead = get_aligned_image_overhead(info, offset); > > > > - return ALIGN(data_size, spl_get_bl_len(info)); > > + if (data_size > ULONG_MAX - overhead) > > + return -EOVERFLOW; > > + data_size += overhead; > > + > > + if (data_size > ULONG_MAX - (spl_get_bl_len(info) - 1)) > > + return -EOVERFLOW; > > + data_size = ALIGN(data_size, spl_get_bl_len(info)); > > + > > + *aligned_size = data_size; > > + > > + return 0; > > } > > > > /** > > @@ -223,17 +233,16 @@ static int load_simple_fit(struct spl_load_info > *info, ulong fit_offset, > > ulong load_addr; > > void *load_ptr; > > void *src; > > - ulong overhead; > > uint8_t image_comp = -1, type = -1; > > const void *data; > > const void *fit = ctx->fit; > > bool external_data = false; > > + int ret; > > > > log_debug("starting\n"); > > if (CONFIG_IS_ENABLED(BOOTMETH_VBE) && > > xpl_get_phase(info) != IH_PHASE_NONE) { > > enum image_phase_t phase; > > - int ret; > > > > ret = fit_image_get_phase(fit, node, &phase); > > /* if the image is for any phase, let's use it */ > > @@ -273,13 +282,19 @@ static int load_simple_fit(struct spl_load_info > *info, ulong fit_offset, > > if (!fit_image_get_data_position(fit, node, &offset)) { > > external_data = true; > > } else if (!fit_image_get_data_offset(fit, node, &offset)) { > > - log_debug("read offset %x = offset from fit %lx\n", > > - offset, (ulong)offset + ctx->ext_data_offset); > > + /* The resulting offset cannot exceed UINT32_MAX */ > > + if (ctx->ext_data_offset > UINT32_MAX - offset) { > > + log_debug("Invalid external data offset: %u\n", > offset); > > + return -EINVAL; > > + } > > + log_debug("read offset %x = offset from fit %x\n", offset, > > + (u32)(offset + ctx->ext_data_offset)); > > offset += ctx->ext_data_offset; > > external_data = true; > > } > > > > if (external_data) { > > + u32 aligned_offset; > > ulong read_offset; > > void *src_ptr; > > > > @@ -300,16 +315,26 @@ static int load_simple_fit(struct spl_load_info > *info, ulong fit_offset, > > * controlled even after fit_config_verify() succeeds. The > > * image hash is only verified after the device read > below, so > > * an oversized value has to be rejected here. > > - * > > - * Bail out before get_aligned_image_size() runs on a > hostile > > - * len: that helper does its arithmetic in int and would > > - * invoke signed-integer overflow on a value close to or > above > > - * INT_MAX. The block-aligned check further down is the > > - * mathematically binding one, since size is len rounded > up to > > - * the device block length. > > */ > > - if ((ulong)len > max_size) > > - goto too_big; > > + ret = get_aligned_image_size(info, len, offset, &size); > > + if (ret) { > > + log_debug("Invalid external data size: %u\n", len); > > + return ret; > > + } > > + > > + if (size > max_size) { > > + log_debug("Image too large: aligned size %lu, max > %lu (data-size %u)\n", > > + size, max_size, len); > > + return -EFBIG; > > + } > > + > > + aligned_offset = get_aligned_image_offset(info, offset); > > + if (aligned_offset > ULONG_MAX - fit_offset) { > > + log_debug("Invalid aligned external data offset: > %u\n", > > + aligned_offset); > > + return -EINVAL; > > + } > > + read_offset = fit_offset + aligned_offset; > > > > if (spl_decompression_enabled() && > > (image_comp == IH_COMP_GZIP || image_comp == > IH_COMP_LZMA)) > > @@ -318,19 +343,6 @@ 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; > > > > - overhead = get_aligned_image_overhead(info, offset); > > - size = get_aligned_image_size(info, length, offset); > > - read_offset = fit_offset + get_aligned_image_offset(info, > > - offset); > > - > > - /* > > - * info->read() transfers the block-aligned size into the > > - * destination, so this is the bound that actually matters; > > - * len was rejected above only to keep this computation > safe. > > - */ > > - if (size > max_size) > > - goto too_big; > > - > > log_debug("reading from offset %x / %lx size %lx to %p: ", > > offset, read_offset, size, src_ptr); > > > > @@ -339,7 +351,7 @@ static int load_simple_fit(struct spl_load_info > *info, ulong fit_offset, > > > > debug("External data: dst=%p, offset=%x, size=%lx\n", > > src_ptr, offset, (unsigned long)length); > > - src = src_ptr + overhead; > > + src = src_ptr + get_aligned_image_overhead(info, offset); > > } else { > > /* Embedded data */ > > if (fit_image_get_emb_data(fit, node, &data, &length)) { > > @@ -401,11 +413,6 @@ static int load_simple_fit(struct spl_load_info > *info, ulong fit_offset, > > upl_add_image(fit, node, load_addr, length); > > > > return 0; > > - > > -too_big: > > - printf("%s: FIT image too large (data-size %u, max %lu)\n", > > - __func__, (u32)len, max_size); > > - return -EFBIG; > > } > > > > static bool os_takes_devicetree(uint8_t os) > > @@ -737,8 +744,9 @@ static int spl_simple_fit_read(struct spl_fit_info > *ctx, > > struct spl_load_info *info, ulong offset, > > const void *fit_header) > > { > > - unsigned long count, size; > > + unsigned long aligned_size, count, size; > > void *buf; > > + int ret; > > > > /* > > * For FIT with external data, figure out where the external images > > @@ -756,8 +764,12 @@ static int spl_simple_fit_read(struct spl_fit_info > *ctx, > > * For FIT with data embedded, data is loaded as part of FIT image. > > * For FIT with external data, data is not loaded in this step. > > */ > > - size = get_aligned_image_size(info, size, 0); > > - buf = board_spl_fit_buffer_addr(size, size, 1); > > + ret = get_aligned_image_size(info, size, 0, &aligned_size); > > + if (ret) { > > + log_debug("Invalid FIT size: %lu\n", size); > > + return ret; > > + } > > + buf = board_spl_fit_buffer_addr(aligned_size, aligned_size, 1); > > if (!buf) { > > /* > > * We assume that none of the board will ever use 0x0 as a > > @@ -767,7 +779,7 @@ static int spl_simple_fit_read(struct spl_fit_info > *ctx, > > return -EIO; > > } > > > > - count = info->read(info, offset, size, buf); > > + count = info->read(info, offset, aligned_size, buf); > > if (!count) { > > /* > > * FIT could not be read. This means we should free the > > @@ -800,7 +812,7 @@ static int spl_simple_fit_read(struct spl_fit_info > *ctx, > > > > ctx->fit = buf; > > debug("fit read offset %lx, size=%lu, dst=%p, count=%lu\n", > > - offset, size, buf, count); > > + offset, aligned_size, buf, count); > > > > return 0; > > } > > diff --git a/test/image/spl_load.c b/test/image/spl_load.c > > index 49bfce15c08..557362ba113 100644 > > --- a/test/image/spl_load.c > > +++ b/test/image/spl_load.c > > @@ -426,6 +426,63 @@ static int spl_test_fit_external_oversize(struct > unit_test_state *uts) > > } > > SPL_TEST(spl_test_fit_external_oversize, 0); > > > > +/* > > + * A data-offset which wraps past UINT32_MAX once the external-data base > > + * offset is added must be rejected before it is used as a read offset. > > + */ > > +static int spl_test_fit_data_offset_overflow(struct unit_test_state > *uts) > > +{ > > + if (!image_supported(FIT_EXTERNAL)) > > + return -EAGAIN; > > + > > + return check_fit_ext_prop(uts, FIT_DATA_OFFSET_PROP, 0xffffffff, 1, > > + spl_test_read, 0, -EINVAL); > > +} > > +SPL_TEST(spl_test_fit_data_offset_overflow, 0); > > + > > +/* > > + * A data-size whose block-aligned read size wraps past ULONG_MAX must > be > > + * rejected. Since data-size is a 32-bit property, the wrap is only > reachable > > + * when ulong is 32 bits wide, so skip the test on other targets. > > + */ > > +static int spl_test_fit_data_size_overflow(struct unit_test_state *uts) > > +{ > > + if (!image_supported(FIT_EXTERNAL) || sizeof(ulong) != 4) > > + return -EAGAIN; > > + > > + return check_fit_ext_prop(uts, FIT_DATA_SIZE_PROP, 0xffffffff, 2, > > + spl_test_read, 0, -EOVERFLOW); > > +} > > +SPL_TEST(spl_test_fit_data_size_overflow, 0); > > + > > +/* Device offset the reader pretends the FIT was loaded from */ > > +static ulong spl_test_fit_offset; > > + > > +static ulong spl_test_read_fit_offset(struct spl_load_info *load, ulong > sector, > > + ulong count, void *buf) > > +{ > > + return spl_test_read(load, sector - spl_test_fit_offset, count, > buf); > > +} > > + > > +/* > > + * An aligned external-data offset which wraps past ULONG_MAX once the > FIT's > > + * offset on the device is added must be rejected before it is used as > a read > > + * offset. > > + */ > > +static int spl_test_fit_read_offset_overflow(struct unit_test_state > *uts) > > +{ > > + if (!image_supported(FIT_EXTERNAL)) > > + return -EAGAIN; > > + > > + /* So that ULONG_MAX - fit_offset < the aligned external-data > offset */ > > + spl_test_fit_offset = ULONG_MAX - 0xfff; > > + > > + return check_fit_ext_prop(uts, FIT_DATA_OFFSET_PROP, 0x1000, 1, > > + spl_test_read_fit_offset, > spl_test_fit_offset, > > + -EINVAL); > > +} > > +SPL_TEST(spl_test_fit_read_offset_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 >
