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

Tony Dinh
2023-02-23T05:23:37Z

Hi Pali,

On Tue, Feb 21, 2023 at 12:22 PM Pali Rohár <pali@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 <mibodhi@gmail.com>

Thanks,
Tony

>
> Signed-off-by: Pali Rohár <pali@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
>