[U-Boot] [PATCH] fdt: Fix FIT header verification in mkimage and conduct same checks as bootm
Simon Glass
sjg at chromium.org
Thu Feb 21 02:47:59 UTC 2019
Hi Jordan,
On Tue, 19 Feb 2019 at 16:40, Jordan Hand <jordanhand22 at gmail.com> wrote:
>
> Signed-off-by: Jordan Hand <jorhand at microsoft.com>
> ---
> FIT header verification in mkimage was treating a return code as a boolean,
> which meant that failures in validating the fit were seen as successes.
>
> Additionally, mkimage was checking all formats to find a header which
> passes validation, rather than using the image type specified to
> mkimage.
>
> checkpatch.pl checks for lines ending with '(' and alignment matching
> open parentheses are ignored to keep with existing coding style.
>
> tools/fit_common.c | 5 ++++-
> tools/imagetool.c | 31 +++++++++++++++++++++++++++++++
> tools/imagetool.h | 15 +++++++++++++++
> tools/mkimage.c | 2 +-
> 4 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/tools/fit_common.c b/tools/fit_common.c
> index d96085eaad..622a9e2a21 100644
> --- a/tools/fit_common.c
> +++ b/tools/fit_common.c
> @@ -26,7 +26,10 @@
> int fit_verify_header(unsigned char *ptr, int image_size,
> struct image_tool_params *params)
> {
> - return fdt_check_header(ptr);
> + if (fdt_check_header(ptr) != EXIT_SUCCESS || !fit_check_format(ptr))
> + return -FDT_ERR_BADSTRUCTURE;
So not a standard error code from ernno.h?
Can you please add a comment to fit_verify_header() to indicate what
it returns? The use of EXIT_SUCCESS suggests that this value is
returned as the program's exit code, and we don't want to return a -ve
exit code.
> +
> + return EXIT_SUCCESS;
> }
>
> int fit_check_image_types(uint8_t type)
> diff --git a/tools/imagetool.c b/tools/imagetool.c
> index b3e628f612..42c6a73925 100644
> --- a/tools/imagetool.c
> +++ b/tools/imagetool.c
> @@ -65,6 +65,37 @@ int imagetool_verify_print_header(
> return retval;
> }
>
> +int imagetool_verify_print_header_by_type(
> + void *ptr,
> + struct stat *sbuf,
> + struct image_type_params *tparams,
> + struct image_tool_params *params)
> +{
> + int retval = tparams->verify_header((unsigned char *)ptr,
> + sbuf->st_size, params);
Can you do:
int retval;
retval = ...
because it is nicer to separate declarations from non-trivial code.
> +
> + if (retval == 0) {
> + /*
> + * Print the image information if verify is
Single space between 'information' and 'if'
> + * successful
> + */
> + if (tparams->print_header) {
> + if (!params->quiet)
> + tparams->print_header(ptr);
> + } else {
> + fprintf(stderr,
> + "%s: print_header undefined for %s\n",
> + params->cmdname, tparams->name);
> + }
> + } else {
> + fprintf(stderr,
> + "%s: verify_header failed for %s with exit code %d\n",
> + params->cmdname, tparams->name, retval);
> + }
> +
> + return retval;
> +}
> +
> int imagetool_save_subimage(
> const char *file_name,
> ulong file_data,
> diff --git a/tools/imagetool.h b/tools/imagetool.h
> index 71471420f9..81f2f60727 100644
> --- a/tools/imagetool.h
> +++ b/tools/imagetool.h
> @@ -179,6 +179,21 @@ int imagetool_verify_print_header(
> struct image_type_params *tparams,
> struct image_tool_params *params);
>
> +/*
> + * imagetool_verify_print_header_by_type() - verifies the image header
> + *
> + * Verify the image_header for the image type given by tparams.
> + * If verification is successful, this prints the respective header.
> + *
Please add docs for the parameters as well.
> + * @return 0 on success, negative if input image format does not match with
> + * the given image type
> + */
> +int imagetool_verify_print_header_by_type(
> + void *ptr,
> + struct stat *sbuf,
> + struct image_type_params *tparams,
> + struct image_tool_params *params);
> +
> /**
> * imagetool_save_subimage - store data into a file
> * @file_name: name of the destination file
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index ea5ed542ab..2899adff81 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -409,7 +409,7 @@ int main(int argc, char **argv)
> * Print the image information for matched image type
> * Returns the error code if not matched
> */
> - retval = imagetool_verify_print_header(ptr, &sbuf,
> + retval = imagetool_verify_print_header_by_type(ptr, &sbuf,
> tparams, ¶ms);
>
> (void) munmap((void *)ptr, sbuf.st_size);
> --
> 2.17.1
>
It would be great to have a test for imagetool in pytest. Not sure if
you want to take that on :-)
Regards,
Simon
More information about the U-Boot
mailing list