| Message ID | 20210914063141.28737-1-matt@traverse.com.au |
|---|---|
| State | New |
| Headers | show |
| Series | image: fit: fix byte order for crc32 hash check | expand |
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
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)); }
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(+) -- 2.30.1