diff mbox series

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

Message ID 20230221201925.9644-16-pali@kernel.org
State Accepted
Commit 7665ed2fa04e0726e142e13bcd77b738e912357f
Delegated to: Stefan Roese
Headers show
Series arm: mvebu: Various fixes | expand

Commit Message

Pali Rohár Feb. 21, 2023, 8:18 p.m. UTC
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.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Tony Dinh Feb. 23, 2023, 5:23 a.m. UTC | #1
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
>
diff mbox series

Patch

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) {