Hi Aristo,
On 2026-09-12T12:44:46, Aristo Chen <[email protected]> wrote:
> tools: mkimage: resolve image-data references between FIT images
>
> An image node may carry an 'image-data' property naming another image
> node under /images whose binary data it shares. This lets several
> configurations load the same payload at different addresses without
> storing it twice, as described in the 'Shared image data' section of
> the Flat Image Tree specification [1].
>
> Resolve the references in mkimage after any external data has been
> imported: the target's data is copied into the referencing node, so the
> later ciphering, hashing and signing steps see a normal image node. The
> property itself is retained in the resolved FIT as a record of the
> sharing relationship. When mkimage re-processes a FIT that already
> contains resolved references, the node's data must match the target's
> data exactly.
>
> A reference is rejected when the target does not exist, when the target
> itself carries an image-data property (chained references), when the
> node's existing data disagrees with the target, or when the node mixes
> image-data with external data properties. Since encryption happens per
> [...]
>
> include/image.h | 1 +
> tools/fit_image.c | 286
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 287 insertions(+)
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> @@ -1169,6 +1169,287 @@ err:
> + ret = fdt_open_into(old_fdt, fdt, size);
> + if (ret) {
> + debug("%s: Failed to expand FIT: %s\n", __func__,
> + fdt_strerror(ret));
> + ret = -EINVAL;
> + goto err_munmap;
> + }
In practice this function should not fail if the DT is corrupt, so
debug() seems good here.
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> @@ -1169,6 +1169,287 @@ err:
> + extra = 0;
> + fdt_for_each_subnode(node, old_fdt, images) {
> + const char *ref;
> + int target, len;
> +
> + ref = fdt_getprop(old_fdt, node, FIT_IMAGE_DATA_PROP, NULL);
> + if (!ref)
> + continue;
> + found = true;
When mkimage -F re-processes an already-resolved FIT, every reference
hits the 'Already resolved' branch and no data is copied, yet the
function still allocates a full copy, runs fdt_open_into(), fdt_pack()
and rewrites the file. Please track whether anything was actually
copied and skip the rewrite when nothing changed.
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> @@ -1169,6 +1169,287 @@ err:
> +/**
> + * fit_check_shared_cipher() - Check cipher settings of images sharing data
> + *
> + * Two image nodes can only share binary data if their cipher settings
The kernel-doc blocks on fit_props_equal(), fit_check_shared_cipher()
and fit_resolve_image_data() are missing @param descriptions and a
Return line. Please spell out the parameters and the return value,
particularly for the cipher one which conflates the source-.its and
re-processed-FIT cases.
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> @@ -1169,6 +1169,287 @@ err:
> + if (fdt_getprop(fdt, node, FIT_DATA_OFFSET_PROP, NULL) ||
> + fdt_getprop(fdt, node, FIT_DATA_POSITION_PROP, NULL) ||
> + fdt_getprop(fdt, node, FIT_DATA_SIZE_PROP, NULL)) {
> + fprintf(stderr,
> + "Error: image '%s': image-data must not be
> combined with external data properties\n",
> + name);
Just to check: fit_import_data() runs before this and, on success,
deletes data-offset/data-position/data-size after importing. So this
check only fires when the source .its declared those properties in a
form fit_import_data() refused to import (e.g. offset without size).
Is that the intent? If you meant to also catch a well-formed
'image-data plus external data' combination in the source, this needs
to happen either before fit_import_data() or by updating
fit_import_data() to skip nodes that carry image-data ... otherwise
the external payload is silently absorbed and the mixing goes
unreported.
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> @@ -1169,6 +1169,287 @@ err:
> + if (fdt_getprop(fdt, ncipher, "iv-name-hint", NULL) ||
> + fdt_getprop(fdt, tcipher, "iv-name-hint", NULL)) {
> + if (!fit_props_equal(fdt, ncipher, tcipher, "iv-name-hint")) {
Since you are adding a new feature that leans heavily on iv-name-hint,
iv and data-size-unciphered, please add FIT_IV_HINT / FIT_IV_PROP /
FIT_DATA_SIZE_UNCIPHERED_PROP alongside FIT_KEY_HINT in
include/image.h and use those here. It keeps the cipher-related
property names in one place and makes future renames tractable.
Regards,
Simon
