Hi He,
On 2026-10-26T14:02:15, He Yong <[email protected]> wrote:
> image: Bound FIT script checks by the image size
>
> FIT verification can follow malformed metadata or external-data offsets
> beyond a buffer whose size is known to the caller.
>
> Pass the known buffer size to fit_check_format() so FIT_FULL_CHECK
> validates the complete structure against the actual buffer. Reject
> external data in all subimages of known-size FITs before verification,
> even when verify is disabled. Keep these checks in the shared parser so
> filesystem and raw-partition scripts receive the same protection. Leave
> unknown-size FIT handling unchanged.
>
> Add C regression coverage with verification enabled and disabled, plus
> source coverage. Clear verify even when a test helper assertion fails.
>
> Signed-off-by: He Yong <[email protected]>
>
> boot/image-board.c | 19 ++++++++-
> test/boot/image.c | 118
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 136 insertions(+), 1 deletion(-)
> diff --git a/boot/image-board.c b/boot/image-board.c
> @@ -1043,11 +1043,28 @@ int image_locate_script(void *buf, int size, const
> char *fit_uname,
> + if (size) {
> + int images, node;
> +
> + /* Reject external data before any verification
> reads. */
> + images = fdt_path_offset(fit_hdr,
> FIT_IMAGES_PATH);
> + fdt_for_each_subnode(node, fit_hdr, images) {
> + if (fdt_getprop(fit_hdr, node,
> + FIT_DATA_OFFSET_PROP,
> NULL) ||
> + fdt_getprop(fit_hdr, node,
> + FIT_DATA_POSITION_PROP,
> NULL)) {
> + puts("External FIT script data
> is not supported\n");
> + return 1;
> + }
> + }
> + }
One thing I'd like to check before this goes in.. bootmeth_script
already passes bflow->size, which is the size of the whole file it
read. A FIT script built with 'mkimage -E' has its external data
inside that file, so it works today and would be rejected after this
patch. It also rejects the FIT if any other subimage (e.g. a kernel)
uses external data, even if that subimage is never used.
Since 'size' is known, it would be better to check that each image's
data-offset/data-position plus data-size lies within it, rejecting
only images that go past the end. fit_image_get_data() already does
similar range checks, so a size-aware version of that might be the
natural place. The raw bootmeth would still be safe, since anything
past fdt_totalsize() would fail the check. What do you think?
If you prefer to keep the blanket rejection, you should describe it in
the commit message and doc as a behaviour change for filesystem
scripts too, not just raw partitions. I'm not sure if it would affect
anyone, but I'm a bit nervous about it.
> diff --git a/boot/image-board.c b/boot/image-board.c
> @@ -1043,11 +1043,28 @@ int image_locate_script(void *buf, int size, const
> char *fit_uname,
> - if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
> + if (fit_check_format(fit_hdr,
> + size ? size : IMAGE_SIZE_INVAL)) {
With FIT_FULL_CHECK disabled, 'size' is ignored, so nothing stops
fdt_totalsize() from exceeding the buffer. It would be worth adding a
plain 'fdt_totalsize(fit_hdr) > size' check when size is non-zero. It
costs nothing and makes the subject line true in all configurations.
Regards,
Simon
