Re: [PATCH RFC u-boot-mvebu 15/59] tools: kwboot: Fix parsing UART image without data checksum

[Tony Dinh]

Hi Pali,

On Tue, Feb 21, 2023 at 12:22 PM Pali Rohár <p...@kernel.org> wrote:
>
> The 32-bit data checksum in UART image is not checked by the BootROM and
> also Marvell tools do not generate it.
>
> So if data checksum stored in UART image does not match calculated checksum
> from the image then treat those checksum bytes as part of the executable
> image code (and not as the checksum) and for compatibility with the rest of
> the code manually insert data checksum into the in-memory image after the
> executable code, without overwriting it.
>
> This should allow to boot UART images generated by Marvell tools.

Tested with GTI Mirabox (Armada 370). Using the stock u-boot image
from NAND mtd0 (nanddump), I've modified the u-boot CONFIG_SYS_PROMPT
text using hexedit. The image checksum should be invalid after that.
New kwboot booted it without problem.

Tested-by: Tony Dinh <mibo...@gmail.com>

Thanks,
Tony

>
> Signed-off-by: Pali Rohár <p...@kernel.org>
> ---
>  tools/kwboot.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 7a7dd5bf3d7b..da840864b565 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1990,8 +1990,18 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>             *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize))
>                 goto err;
>
> -       if (kwboot_img_csum32(img) != *kwboot_img_csum32_ptr(img))
> -               goto err;
> +       /*
> +        * The 32-bit data checksum is optional for UART image. If it is not
> +        * present (checksum detected as invalid) then grow data part of the
> +        * image for the checksum, so it can be inserted there.
> +        */
> +       if (kwboot_img_csum32(img) != *kwboot_img_csum32_ptr(img)) {
> +               if (hdr->blockid != IBR_HDR_UART_ID) {
> +                       fprintf(stderr, "Image has invalid data checksum\n");
> +                       goto err;
> +               }
> +               kwboot_img_grow_data_right(img, size, sizeof(uint32_t));
> +       }
>
>         is_secure = kwboot_img_is_secure(img);
>
> @@ -2256,6 +2266,7 @@ main(int argc, char **argv)
>                                  KWBOOT_XM_BLKSZ +
>                                  sizeof(kwboot_baud_code) +
>                                  sizeof(kwboot_baud_code_data_jump) +
> +                                sizeof(uint32_t) +
>                                  KWBOOT_XM_BLKSZ;
>
>         if (imgpath) {
> --
> 2.20.1
>