[PATCH v2 4/4] iminfo: also verify signatures

[Simon Glass]

Hi Ludwig,

On 2026-04-30T12:25:59, Ludwig Nussel <ludwig.nussel at siemens.com> wrote:
> iminfo: also verify signatures
>
> The iminfo command already verifies hashes of images. This change also
> verifies signatures of configurations if enabled.
>
> Signed-off-by: Ludwig Nussel <ludwig.nussel at siemens.com>
>
> boot/image-fit.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  cmd/bootm.c      |  7 +++++++
>  include/image.h  |  1 +
>  3 files changed, 56 insertions(+)

> diff --git a/boot/image-fit.c b/boot/image-fit.c
> @@ -1512,6 +1512,54 @@ int fit_all_image_verify(const void *fit)
> +     /* Find images parent node offset */
> +     confs_noffset = fdt_path_offset(fit, FIT_CONFS_PATH);
> +     if (confs_noffset < 0) {
> +             printf("Can't find configurations parent node '%s' (%s)\n",
> +                    FIT_IMAGES_PATH, fdt_strerror(confs_noffset));
> +             return confs_noffset;
> +     }

The error message passes FIT_IMAGES_PATH but the lookup is
FIT_CONFS_PATH, so a missing /configurations node will report
'/images'. Also the preceding comment is a stale copy from
fit_all_image_verify() and should refer to configurations.

> diff --git a/boot/image-fit.c b/boot/image-fit.c
> @@ -1512,6 +1512,54 @@ int fit_all_image_verify(const void *fit)
> +     /* Process all config subnodes, check hashes for each */
> +     printf("## Checking configuration signatures ...\n");

Another stale comment - it's signatures here, not hashes.

> diff --git a/boot/image-fit.c b/boot/image-fit.c
> @@ -1512,6 +1512,54 @@ int fit_all_image_verify(const void *fit)
> + * fit_all_configurations_verify() iterates over all configurations
> + * in the FIT and checks the signatures. Returns success if all
> + * configurations have valid signatures. See documentation at
> + * fit_config_verify_required_keys() resp fit_config_verify_key().

s/resp/or/

While here, the kernel-doc would be clearer if it stated that -ENOENT
is returned when no configurations are present, given the r = -ENOENT
seed.

> diff --git a/boot/image-fit.c b/boot/image-fit.c
> @@ -1512,6 +1512,54 @@ int fit_all_image_verify(const void *fit)
> +     fdt_for_each_subnode(noffset, fit, confs_noffset) {
> +             int ret;
> +
> +             printf("   %s ... ", fit_get_name(fit, noffset, NULL));
> +             ret = fit_config_verify(fit, noffset);
> +             if (ret) {
> +                     r = ret;
> +                     continue;
> +             }
> +             /* valid config found */
> +             if (r == -ENOENT)
> +                     r = 0;
> +             puts('OK\n');
> +     }

Patch 2 just added log_err()/log_info() centrally — please use those
(and log_debug() where appropriate) rather than raw printf()/puts(),
since that is the direction of the rest of the series. Also, on a
failing config the "%s ... " line is left dangling without a newline;
please emit a clear FAIL marker in the ret != 0 branch.

> diff --git a/cmd/bootm.c b/cmd/bootm.c
> @@ -335,6 +335,13 @@ static int image_info(ulong addr)
> +             if (CONFIG_IS_ENABLED(FIT_SIGNATURE_REQUIRED) &&
> +                 fit_all_configurations_verify(hdr) != 0) {
> +                     puts("Signature verification failed!\n");
> +                     unmap_sysmem(hdr);
> +                     return 1;
> +             }
> +

The subject says iminfo "also verifies signatures", but verification
only runs when FIT_SIGNATURE_REQUIRED is set. I'd expect iminfo to
attempt verification whenever FIT_SIGNATURE is enabled so the user
sees the result, and only treat failure as fatal when REQUIRED is set
- otherwise users on the legacy path lose the diagnostic value. What
do you think?

> diff --git a/include/image.h b/include/image.h
> @@ -1355,6 +1355,7 @@ static inline int fit_config_verify(const void *fit, int conf_noffset)
>  }
>  #endif
>  int fit_all_image_verify(const void *fit);
> +int fit_all_configurations_verify(const void *fit);

Please add a function comment.

fit_all_configurations_verify() only does meaningful work when
FIT_SIGNATURE is enabled, otherwise fit_config_verify() is a stub
returning 0 and every config falsely reports OK. Please mirror the
fit_config_verify() pattern with a stub for the !FIT_SIGNATURE case,
or at least guard the body so a CONFIG_FIT-only build doesn't print
misleading OK lines.

Regards,
Simon