Re: [PATCH v3 1/2] cmd: mvebu/bubt: Check for A38x image data checksum

Stefan Roese
2022-09-13T06:51:11Z

On 11.09.22 13:26, Pali Rohár wrote:
> On Tuesday 23 August 2022 14:52:23 Pali Rohár wrote:
>> Currently for A38x image is checked only header checksum.
>> So check also for image data checksum to prevent flashing broken image.
>>
>> Signed-off-by: Pali Rohár <pali@kernel.org>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>>
>> ---
>> Changes in v3:
>> * Compile fix (move another code chunk from patch 2/2 to 1/2)
>>
>> Changes in v2:
>> * Compile fix (move code chunk from patch 2/2 to 1/2)
>> ---
>
> Hello Stefan! Is V3 version OK now?

Looks good.

Thanks,
Stefan

>> cmd/mvebu/bubt.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
>> index 2136af64163c..a97e5ce38a5e 100644
>> --- a/cmd/mvebu/bubt.c
>> +++ b/cmd/mvebu/bubt.c
>> @@ -688,9 +688,25 @@ static uint8_t image_checksum8(const void *start, size_t len)
>> return csum;
>> }
>>
>> +static uint32_t image_checksum32(const void *start, size_t len)
>> +{
>> + u32 csum = 0;
>> + const u32 *p = start;
>> +
>> + while (len) {
>> + csum += *p;
>> + ++p;
>> + len -= sizeof(u32);
>> + }
>> +
>> + return csum;
>> +}
>> +
>> static int check_image_header(void)
>> {
>> u8 checksum;
>> + u32 checksum32, exp_checksum32;
>> + u32 offset, size;
>> const struct a38x_main_hdr_v1 *hdr =
>> (struct a38x_main_hdr_v1 *)get_load_addr();
>> const size_t image_size = a38x_header_size(hdr);
>> @@ -701,11 +717,39 @@ static int check_image_header(void)
>> checksum = image_checksum8(hdr, image_size);
>> checksum -= hdr->checksum;
>> if (checksum != hdr->checksum) {
>> - printf("Error: Bad A38x image checksum. 0x%x != 0x%x\n",
>> + printf("Error: Bad A38x image header checksum. 0x%x != 0x%x\n",
>> checksum, hdr->checksum);
>> return -ENOEXEC;
>> }
>>
>> + offset = le32_to_cpu(hdr->srcaddr);
>> + size = le32_to_cpu(hdr->blocksize);
>> +
>> + if (hdr->blockid == 0x78) { /* SATA id */
>> + if (offset < 1) {
>> + printf("Error: Bad A38x image srcaddr.\n");
>> + return -ENOEXEC;
>> + }
>> + offset -= 1;
>> + offset *= 512;
>> + }
>> +
>> + if (hdr->blockid == 0xAE) /* SDIO id */
>> + offset *= 512;
>> +
>> + if (offset % 4 != 0 || size < 4 || size % 4 != 0) {
>> + printf("Error: Bad A38x image blocksize.\n");
>> + return -ENOEXEC;
>> + }
>> +
>> + checksum32 = image_checksum32((u8 *)hdr + offset, size - 4);
>> + exp_checksum32 = *(u32 *)((u8 *)hdr + offset + size - 4);
>> + if (checksum32 != exp_checksum32) {
>> + printf("Error: Bad A38x image data checksum. 0x%08x != 0x%08x\n",
>> + checksum32, exp_checksum32);
>> + return -ENOEXEC;
>> + }
>> +
>> printf("Image checksum...OK!\n");
>> return 0;
>> }
>> --
>> 2.20.1
>>

Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de