[PATCH] image: fit: fix byte order for crc32 hash check

[PATCH] image: fit: fix byte order for crc32 hash check

[Mathew McBride]

In 92055e138f ("image: Drop if/elseif hash selection in calculate_hash()")
the FIT image hash verification code was simplified to use the hash API.

This broke verification of CRC32 hash FIT images (e.g NXP MC firmware)
as the hash API crc32 calculates in big endian (refer commit
74a18ee8a563, "crc32: Correct endianness of crc32 result"), whereas
the previous call directly to crc32_wd does not do a byteswap.

Example:
dumpimage -l qoriq-mc-binary/ls1088a/mc_ls1088a_10.29.1.itb
FIT description: MC Firmware
Created:         Fri Aug 27 14:14:32 2021
...
  Hash algo:    crc32
  Hash value:   c09d959c
Was returning FB9D3710 to fit_image_check_hash instead of C09D959C

Correct the byte order in the crc32_uimage_fixup so the little endian
values are returned.

Fixes: 92055e138f "image: Drop if/elseif hash selection in calculate_hash()"
Signed-off-by: Mathew McBride <matt@traverse.com.au>
---
 common/image-fit.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/common/image-fit.c b/common/image-fit.c
index 92d9141bcd..97b4f0b4b2 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1195,6 +1195,11 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp)
 
 static void crc32_uimage_fixup(void *value)
 {
+	/* In the CRC32 case, value will be byte swapped by the hash library
+	 * function, but the FIT is specified (and checked) in little-endian
+	 */
+	*((uint32_t *)value) = ntohl(*((uint32_t *)value));
+
 	/* TODO: In C, this type punning is undefined behavior: */
 	*((uint32_t *)value) = cpu_to_uimage(*((uint32_t *)value));
 }
-- 
2.30.1

Re: [PATCH] image: fit: fix byte order for crc32 hash check

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

On Tue, Sep 14, 2021 at 06:31:41AM +0000, Mathew McBride wrote:

> In 92055e138f ("image: Drop if/elseif hash selection in calculate_hash()")
> the FIT image hash verification code was simplified to use the hash API.
> 
> This broke verification of CRC32 hash FIT images (e.g NXP MC firmware)
> as the hash API crc32 calculates in big endian (refer commit
> 74a18ee8a563, "crc32: Correct endianness of crc32 result"), whereas
> the previous call directly to crc32_wd does not do a byteswap.
> 
> Example:
> dumpimage -l qoriq-mc-binary/ls1088a/mc_ls1088a_10.29.1.itb
> FIT description: MC Firmware
> Created:         Fri Aug 27 14:14:32 2021
> ...
>   Hash algo:    crc32
>   Hash value:   c09d959c
> Was returning FB9D3710 to fit_image_check_hash instead of C09D959C
> 
> Correct the byte order in the crc32_uimage_fixup so the little endian
> values are returned.
> 
> Fixes: 92055e138f "image: Drop if/elseif hash selection in calculate_hash()"
> Signed-off-by: Mathew McBride <matt@traverse.com.au>

Good catch.  However:
   sandbox:  w+   sandbox
+(sandbox)   *((uint32_t *)value) = ntohl(*((uint32_t *)value));
+(sandbox)                          ^~~~~
+(sandbox)                          atoll
w+(sandbox) tools/../common/image-fit.c: In function ‘crc32_uimage_fixup’:
w+(sandbox) tools/../common/image-fit.c:1201:25: warning: implicit declaration of function ‘ntohl’; did you mean ‘atoll’? [-Wimplicit-function-declaration]

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]