[PATCH RFC u-boot-mvebu 19/59] tools: kwboot: Show image type and error parsing reasons

[Tony Dinh]

On Tue, Feb 21, 2023 at 12:22 PM Pali Rohár <pali at kernel.org> wrote:
>
> Show image type and version during parsing of kwbimage.
> And show reasons in error messages when parsing failed.
> This can help to debug issues with invalid images.
>
> Signed-off-by: Pali Rohár <pali at kernel.org>
> ---
>  tools/kwboot.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index cb31d5b858ce..7c666486f31f 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1976,6 +1976,21 @@ _inject_baudrate_change_code(void *img, size_t *size, int for_data,
>         }
>  }
>
> +static const char *
> +kwboot_img_type(uint8_t blockid)
> +{
> +       switch (blockid) {
> +       case IBR_HDR_I2C_ID: return "I2C";
> +       case IBR_HDR_SPI_ID: return "SPI";
> +       case IBR_HDR_NAND_ID: return "NAND";
> +       case IBR_HDR_SATA_ID: return "SATA";
> +       case IBR_HDR_PEX_ID: return "PEX";
> +       case IBR_HDR_UART_ID: return "UART";
> +       case IBR_HDR_SDIO_ID: return "SDIO";
> +       default: return "unknown";
> +       }
> +}
> +
>  static int
>  kwboot_img_patch(void *img, size_t *size, int baudrate)
>  {
> @@ -1989,8 +2004,10 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>
>         hdr = img;
>
> -       if (*size < sizeof(struct main_hdr_v1))
> +       if (*size < sizeof(struct main_hdr_v1)) {
> +               fprintf(stderr, "Invalid image header size\n");
>                 goto err;
> +       }
>
>         image_ver = kwbimage_version(img);
>         if (image_ver != 0 && image_ver != 1) {
> @@ -2000,12 +2017,18 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>
>         hdrsz = kwbheader_size(hdr);
>
> -       if (*size < hdrsz)
> +       if (*size < hdrsz) {
> +               fprintf(stderr, "Invalid image header size\n");
>                 goto err;
> +       }
> +
> +       kwboot_printv("Detected kwbimage v%d with %s boot signature\n", image_ver, kwboot_img_type(hdr->blockid));
>
>         csum = kwboot_hdr_csum8(hdr) - hdr->checksum;
> -       if (csum != hdr->checksum)
> +       if (csum != hdr->checksum) {
> +               fprintf(stderr, "Image has invalid header checksum stored in image header\n");
>                 goto err;
> +       }
>
>         srcaddr = le32_to_cpu(hdr->srcaddr);
>
> @@ -2028,9 +2051,15 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>                 break;
>         }
>
> -       if (hdrsz > le32_to_cpu(hdr->srcaddr) ||
> -           *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize))
> +       if (hdrsz > le32_to_cpu(hdr->srcaddr)) {
> +               fprintf(stderr, "Image has invalid data offset stored in image header\n");
> +               goto err;
> +       }
> +
> +       if (*size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize)) {
> +               fprintf(stderr, "Image has invalid data size stored in image header\n");
>                 goto err;
> +       }
>
>         for_each_opt_hdr_v1 (ohdr, hdr) {
>                 if (!opt_hdr_v1_valid_size(ohdr, (const uint8_t *)hdr + hdrsz)) {
> --
> 2.20.1
>
Nominal cases (no error) with Armada 385 and KIrkwood 6192:

Detected kwbimage v1 with SPI boot signature
Detected kwbimage v0 with NAND boot signature

Tested-by: Tony Dinh <mibodhi at gmail.com>

All the best,
Tony