From: Andreas Bolsch Date: Sat, 28 Oct 2017 17:39:35 +0000 (+0200) Subject: Handle improperly build image files gracefully X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=445dc23eb51bc2d7f4466392d36d40fcecf54f65;p=openocd Handle improperly build image files gracefully Images build improperly (by simply concatenating separate images) were accepted, but anything after the first end-of-file record *silently* ignored. Now emit warning for intel and motorola images upon non-whitespace after first end-of-file record but continue reading anyway. ST ships some images broken that way in their CubeMX packages ... Change-Id: I0c5d08fa90070fed11fb805c5f0dc39817048176 Signed-off-by: Andreas Bolsch Reviewed-on: http://openocd.zylin.com/4281 Tested-by: jenkins Reviewed-by: Andreas Fritiofson Reviewed-by: Tomas Vanek --- diff --git a/src/helper/fileio.c b/src/helper/fileio.c index 47494dfc..9dd57e72 100644 --- a/src/helper/fileio.c +++ b/src/helper/fileio.c @@ -153,6 +153,11 @@ int fileio_close(struct fileio *fileio) return retval; } +int fileio_feof(struct fileio *fileio) +{ + return feof(fileio->file); +} + int fileio_seek(struct fileio *fileio, size_t position) { int retval; diff --git a/src/helper/fileio.h b/src/helper/fileio.h index ae4a3ecf..02592e28 100644 --- a/src/helper/fileio.h +++ b/src/helper/fileio.h @@ -46,6 +46,7 @@ struct fileio; int fileio_open(struct fileio **fileio, const char *url, enum fileio_access access_type, enum fileio_type type); int fileio_close(struct fileio *fileio); +int fileio_feof(struct fileio *fileio); int fileio_seek(struct fileio *fileio, size_t position); int fileio_fgets(struct fileio *fileio, size_t size, void *buffer); diff --git a/src/target/image.c b/src/target/image.c index f97d9040..9f56bea0 100644 --- a/src/target/image.c +++ b/src/target/image.c @@ -121,8 +121,9 @@ static int image_ihex_buffer_complete_inner(struct image *image, { struct image_ihex *ihex = image->type_private; struct fileio *fileio = ihex->fileio; - uint32_t full_address = 0x0; + uint32_t full_address; uint32_t cooked_bytes; + bool end_rec = false; int i; /* we can't determine the number of sections that we'll have to create ahead of time, @@ -137,175 +138,190 @@ static int image_ihex_buffer_complete_inner(struct image *image, ihex->buffer = malloc(filesize >> 1); cooked_bytes = 0x0; image->num_sections = 0; - section[image->num_sections].private = &ihex->buffer[cooked_bytes]; - section[image->num_sections].base_address = 0x0; - section[image->num_sections].size = 0x0; - section[image->num_sections].flags = 0; - - while (fileio_fgets(fileio, 1023, lpszLine) == ERROR_OK) { - uint32_t count; - uint32_t address; - uint32_t record_type; - uint32_t checksum; - uint8_t cal_checksum = 0; - size_t bytes_read = 0; - - if (lpszLine[0] == '#') - continue; - - if (sscanf(&lpszLine[bytes_read], ":%2" SCNx32 "%4" SCNx32 "%2" SCNx32, &count, - &address, &record_type) != 3) - return ERROR_IMAGE_FORMAT_ERROR; - bytes_read += 9; - - cal_checksum += (uint8_t)count; - cal_checksum += (uint8_t)(address >> 8); - cal_checksum += (uint8_t)address; - cal_checksum += (uint8_t)record_type; - - if (record_type == 0) { /* Data Record */ - if ((full_address & 0xffff) != address) { - /* we encountered a nonconsecutive location, create a new section, - * unless the current section has zero size, in which case this specifies - * the current section's base address - */ - if (section[image->num_sections].size != 0) { - image->num_sections++; - if (image->num_sections >= IMAGE_MAX_SECTIONS) { - /* too many sections */ - LOG_ERROR("Too many sections found in IHEX file"); - return ERROR_IMAGE_FORMAT_ERROR; + + while (!fileio_feof(fileio)) { + full_address = 0x0; + section[image->num_sections].private = &ihex->buffer[cooked_bytes]; + section[image->num_sections].base_address = 0x0; + section[image->num_sections].size = 0x0; + section[image->num_sections].flags = 0; + + while (fileio_fgets(fileio, 1023, lpszLine) == ERROR_OK) { + uint32_t count; + uint32_t address; + uint32_t record_type; + uint32_t checksum; + uint8_t cal_checksum = 0; + size_t bytes_read = 0; + + /* skip comments and blank lines */ + if ((lpszLine[0] == '#') || (strlen(lpszLine + strspn(lpszLine, "\n\t\r ")) == 0)) + continue; + + if (sscanf(&lpszLine[bytes_read], ":%2" SCNx32 "%4" SCNx32 "%2" SCNx32, &count, + &address, &record_type) != 3) + return ERROR_IMAGE_FORMAT_ERROR; + bytes_read += 9; + + cal_checksum += (uint8_t)count; + cal_checksum += (uint8_t)(address >> 8); + cal_checksum += (uint8_t)address; + cal_checksum += (uint8_t)record_type; + + if (record_type == 0) { /* Data Record */ + if ((full_address & 0xffff) != address) { + /* we encountered a nonconsecutive location, create a new section, + * unless the current section has zero size, in which case this specifies + * the current section's base address + */ + if (section[image->num_sections].size != 0) { + image->num_sections++; + if (image->num_sections >= IMAGE_MAX_SECTIONS) { + /* too many sections */ + LOG_ERROR("Too many sections found in IHEX file"); + return ERROR_IMAGE_FORMAT_ERROR; + } + section[image->num_sections].size = 0x0; + section[image->num_sections].flags = 0; + section[image->num_sections].private = + &ihex->buffer[cooked_bytes]; } - section[image->num_sections].size = 0x0; - section[image->num_sections].flags = 0; - section[image->num_sections].private = - &ihex->buffer[cooked_bytes]; + section[image->num_sections].base_address = + (full_address & 0xffff0000) | address; + full_address = (full_address & 0xffff0000) | address; } - section[image->num_sections].base_address = - (full_address & 0xffff0000) | address; - full_address = (full_address & 0xffff0000) | address; - } - - while (count-- > 0) { - unsigned value; - sscanf(&lpszLine[bytes_read], "%2x", &value); - ihex->buffer[cooked_bytes] = (uint8_t)value; - cal_checksum += (uint8_t)ihex->buffer[cooked_bytes]; - bytes_read += 2; - cooked_bytes += 1; - section[image->num_sections].size += 1; - full_address++; - } - } else if (record_type == 1) { /* End of File Record */ - /* finish the current section */ - image->num_sections++; - - /* copy section information */ - image->sections = malloc(sizeof(struct imagesection) * image->num_sections); - for (i = 0; i < image->num_sections; i++) { - image->sections[i].private = section[i].private; - image->sections[i].base_address = section[i].base_address; - image->sections[i].size = section[i].size; - image->sections[i].flags = section[i].flags; - } - return ERROR_OK; - } else if (record_type == 2) { /* Linear Address Record */ - uint16_t upper_address; - - sscanf(&lpszLine[bytes_read], "%4hx", &upper_address); - cal_checksum += (uint8_t)(upper_address >> 8); - cal_checksum += (uint8_t)upper_address; - bytes_read += 4; + while (count-- > 0) { + unsigned value; + sscanf(&lpszLine[bytes_read], "%2x", &value); + ihex->buffer[cooked_bytes] = (uint8_t)value; + cal_checksum += (uint8_t)ihex->buffer[cooked_bytes]; + bytes_read += 2; + cooked_bytes += 1; + section[image->num_sections].size += 1; + full_address++; + } + } else if (record_type == 1) { /* End of File Record */ + /* finish the current section */ + image->num_sections++; + + /* copy section information */ + image->sections = malloc(sizeof(struct imagesection) * image->num_sections); + for (i = 0; i < image->num_sections; i++) { + image->sections[i].private = section[i].private; + image->sections[i].base_address = section[i].base_address; + image->sections[i].size = section[i].size; + image->sections[i].flags = section[i].flags; + } - if ((full_address >> 4) != upper_address) { - /* we encountered a nonconsecutive location, create a new section, - * unless the current section has zero size, in which case this specifies - * the current section's base address - */ - if (section[image->num_sections].size != 0) { - image->num_sections++; - if (image->num_sections >= IMAGE_MAX_SECTIONS) { - /* too many sections */ - LOG_ERROR("Too many sections found in IHEX file"); - return ERROR_IMAGE_FORMAT_ERROR; + end_rec = true; + break; + } else if (record_type == 2) { /* Linear Address Record */ + uint16_t upper_address; + + sscanf(&lpszLine[bytes_read], "%4hx", &upper_address); + cal_checksum += (uint8_t)(upper_address >> 8); + cal_checksum += (uint8_t)upper_address; + bytes_read += 4; + + if ((full_address >> 4) != upper_address) { + /* we encountered a nonconsecutive location, create a new section, + * unless the current section has zero size, in which case this specifies + * the current section's base address + */ + if (section[image->num_sections].size != 0) { + image->num_sections++; + if (image->num_sections >= IMAGE_MAX_SECTIONS) { + /* too many sections */ + LOG_ERROR("Too many sections found in IHEX file"); + return ERROR_IMAGE_FORMAT_ERROR; + } + section[image->num_sections].size = 0x0; + section[image->num_sections].flags = 0; + section[image->num_sections].private = + &ihex->buffer[cooked_bytes]; } - section[image->num_sections].size = 0x0; - section[image->num_sections].flags = 0; - section[image->num_sections].private = - &ihex->buffer[cooked_bytes]; + section[image->num_sections].base_address = + (full_address & 0xffff) | (upper_address << 4); + full_address = (full_address & 0xffff) | (upper_address << 4); } - section[image->num_sections].base_address = - (full_address & 0xffff) | (upper_address << 4); - full_address = (full_address & 0xffff) | (upper_address << 4); - } - } else if (record_type == 3) { /* Start Segment Address Record */ - uint32_t dummy; - - /* "Start Segment Address Record" will not be supported - * but we must consume it, and do not create an error. */ - while (count-- > 0) { - sscanf(&lpszLine[bytes_read], "%2" SCNx32, &dummy); - cal_checksum += (uint8_t)dummy; - bytes_read += 2; - } - } else if (record_type == 4) { /* Extended Linear Address Record */ - uint16_t upper_address; - - sscanf(&lpszLine[bytes_read], "%4hx", &upper_address); - cal_checksum += (uint8_t)(upper_address >> 8); - cal_checksum += (uint8_t)upper_address; - bytes_read += 4; - - if ((full_address >> 16) != upper_address) { - /* we encountered a nonconsecutive location, create a new section, - * unless the current section has zero size, in which case this specifies - * the current section's base address - */ - if (section[image->num_sections].size != 0) { - image->num_sections++; - if (image->num_sections >= IMAGE_MAX_SECTIONS) { - /* too many sections */ - LOG_ERROR("Too many sections found in IHEX file"); - return ERROR_IMAGE_FORMAT_ERROR; + } else if (record_type == 3) { /* Start Segment Address Record */ + uint32_t dummy; + + /* "Start Segment Address Record" will not be supported + * but we must consume it, and do not create an error. */ + while (count-- > 0) { + sscanf(&lpszLine[bytes_read], "%2" SCNx32, &dummy); + cal_checksum += (uint8_t)dummy; + bytes_read += 2; + } + } else if (record_type == 4) { /* Extended Linear Address Record */ + uint16_t upper_address; + + sscanf(&lpszLine[bytes_read], "%4hx", &upper_address); + cal_checksum += (uint8_t)(upper_address >> 8); + cal_checksum += (uint8_t)upper_address; + bytes_read += 4; + + if ((full_address >> 16) != upper_address) { + /* we encountered a nonconsecutive location, create a new section, + * unless the current section has zero size, in which case this specifies + * the current section's base address + */ + if (section[image->num_sections].size != 0) { + image->num_sections++; + if (image->num_sections >= IMAGE_MAX_SECTIONS) { + /* too many sections */ + LOG_ERROR("Too many sections found in IHEX file"); + return ERROR_IMAGE_FORMAT_ERROR; + } + section[image->num_sections].size = 0x0; + section[image->num_sections].flags = 0; + section[image->num_sections].private = + &ihex->buffer[cooked_bytes]; } - section[image->num_sections].size = 0x0; - section[image->num_sections].flags = 0; - section[image->num_sections].private = - &ihex->buffer[cooked_bytes]; + section[image->num_sections].base_address = + (full_address & 0xffff) | (upper_address << 16); + full_address = (full_address & 0xffff) | (upper_address << 16); } - section[image->num_sections].base_address = - (full_address & 0xffff) | (upper_address << 16); - full_address = (full_address & 0xffff) | (upper_address << 16); + } else if (record_type == 5) { /* Start Linear Address Record */ + uint32_t start_address; + + sscanf(&lpszLine[bytes_read], "%8" SCNx32, &start_address); + cal_checksum += (uint8_t)(start_address >> 24); + cal_checksum += (uint8_t)(start_address >> 16); + cal_checksum += (uint8_t)(start_address >> 8); + cal_checksum += (uint8_t)start_address; + bytes_read += 8; + + image->start_address_set = 1; + image->start_address = be_to_h_u32((uint8_t *)&start_address); + } else { + LOG_ERROR("unhandled IHEX record type: %i", (int)record_type); + return ERROR_IMAGE_FORMAT_ERROR; } - } else if (record_type == 5) { /* Start Linear Address Record */ - uint32_t start_address; - - sscanf(&lpszLine[bytes_read], "%8" SCNx32, &start_address); - cal_checksum += (uint8_t)(start_address >> 24); - cal_checksum += (uint8_t)(start_address >> 16); - cal_checksum += (uint8_t)(start_address >> 8); - cal_checksum += (uint8_t)start_address; - bytes_read += 8; - - image->start_address_set = 1; - image->start_address = be_to_h_u32((uint8_t *)&start_address); - } else { - LOG_ERROR("unhandled IHEX record type: %i", (int)record_type); - return ERROR_IMAGE_FORMAT_ERROR; - } - sscanf(&lpszLine[bytes_read], "%2" SCNx32, &checksum); + sscanf(&lpszLine[bytes_read], "%2" SCNx32, &checksum); - if ((uint8_t)checksum != (uint8_t)(~cal_checksum + 1)) { - /* checksum failed */ - LOG_ERROR("incorrect record checksum found in IHEX file"); - return ERROR_IMAGE_CHECKSUM; + if ((uint8_t)checksum != (uint8_t)(~cal_checksum + 1)) { + /* checksum failed */ + LOG_ERROR("incorrect record checksum found in IHEX file"); + return ERROR_IMAGE_CHECKSUM; + } + + if (end_rec) { + end_rec = false; + LOG_WARNING("continuing after end-of-file record: %.40s", lpszLine); + } } } - LOG_ERROR("premature end of IHEX file, no end-of-file record found"); - return ERROR_IMAGE_FORMAT_ERROR; + if (end_rec) + return ERROR_OK; + else { + LOG_ERROR("premature end of IHEX file, no matching end-of-file record found"); + return ERROR_IMAGE_FORMAT_ERROR; + } } /** @@ -510,8 +526,9 @@ static int image_mot_buffer_complete_inner(struct image *image, { struct image_mot *mot = image->type_private; struct fileio *fileio = mot->fileio; - uint32_t full_address = 0x0; + uint32_t full_address; uint32_t cooked_bytes; + bool end_rec = false; int i; /* we can't determine the number of sections that we'll have to create ahead of time, @@ -526,140 +543,158 @@ static int image_mot_buffer_complete_inner(struct image *image, mot->buffer = malloc(filesize >> 1); cooked_bytes = 0x0; image->num_sections = 0; - section[image->num_sections].private = &mot->buffer[cooked_bytes]; - section[image->num_sections].base_address = 0x0; - section[image->num_sections].size = 0x0; - section[image->num_sections].flags = 0; - - while (fileio_fgets(fileio, 1023, lpszLine) == ERROR_OK) { - uint32_t count; - uint32_t address; - uint32_t record_type; - uint32_t checksum; - uint8_t cal_checksum = 0; - uint32_t bytes_read = 0; - - /* get record type and record length */ - if (sscanf(&lpszLine[bytes_read], "S%1" SCNx32 "%2" SCNx32, &record_type, - &count) != 2) - return ERROR_IMAGE_FORMAT_ERROR; - - bytes_read += 4; - cal_checksum += (uint8_t)count; - - /* skip checksum byte */ - count -= 1; - - if (record_type == 0) { - /* S0 - starting record (optional) */ - int iValue; - - while (count-- > 0) { - sscanf(&lpszLine[bytes_read], "%2x", &iValue); - cal_checksum += (uint8_t)iValue; - bytes_read += 2; - } - } else if (record_type >= 1 && record_type <= 3) { - switch (record_type) { - case 1: - /* S1 - 16 bit address data record */ - sscanf(&lpszLine[bytes_read], "%4" SCNx32, &address); - cal_checksum += (uint8_t)(address >> 8); - cal_checksum += (uint8_t)address; - bytes_read += 4; - count -= 2; - break; - - case 2: - /* S2 - 24 bit address data record */ - sscanf(&lpszLine[bytes_read], "%6" SCNx32, &address); - cal_checksum += (uint8_t)(address >> 16); - cal_checksum += (uint8_t)(address >> 8); - cal_checksum += (uint8_t)address; - bytes_read += 6; - count -= 3; - break; - - case 3: - /* S3 - 32 bit address data record */ - sscanf(&lpszLine[bytes_read], "%8" SCNx32, &address); - cal_checksum += (uint8_t)(address >> 24); - cal_checksum += (uint8_t)(address >> 16); - cal_checksum += (uint8_t)(address >> 8); - cal_checksum += (uint8_t)address; - bytes_read += 8; - count -= 4; - break; - } + while (!fileio_feof(fileio)) { + full_address = 0x0; + section[image->num_sections].private = &mot->buffer[cooked_bytes]; + section[image->num_sections].base_address = 0x0; + section[image->num_sections].size = 0x0; + section[image->num_sections].flags = 0; + + while (fileio_fgets(fileio, 1023, lpszLine) == ERROR_OK) { + uint32_t count; + uint32_t address; + uint32_t record_type; + uint32_t checksum; + uint8_t cal_checksum = 0; + uint32_t bytes_read = 0; + + /* skip comments and blank lines */ + if ((lpszLine[0] == '#') || (strlen(lpszLine + strspn(lpszLine, "\n\t\r ")) == 0)) + continue; + + /* get record type and record length */ + if (sscanf(&lpszLine[bytes_read], "S%1" SCNx32 "%2" SCNx32, &record_type, + &count) != 2) + return ERROR_IMAGE_FORMAT_ERROR; + + bytes_read += 4; + cal_checksum += (uint8_t)count; - if (full_address != address) { - /* we encountered a nonconsecutive location, create a new section, - * unless the current section has zero size, in which case this specifies - * the current section's base address - */ - if (section[image->num_sections].size != 0) { - image->num_sections++; - section[image->num_sections].size = 0x0; - section[image->num_sections].flags = 0; - section[image->num_sections].private = - &mot->buffer[cooked_bytes]; + /* skip checksum byte */ + count -= 1; + + if (record_type == 0) { + /* S0 - starting record (optional) */ + int iValue; + + while (count-- > 0) { + sscanf(&lpszLine[bytes_read], "%2x", &iValue); + cal_checksum += (uint8_t)iValue; + bytes_read += 2; } - section[image->num_sections].base_address = address; - full_address = address; - } + } else if (record_type >= 1 && record_type <= 3) { + switch (record_type) { + case 1: + /* S1 - 16 bit address data record */ + sscanf(&lpszLine[bytes_read], "%4" SCNx32, &address); + cal_checksum += (uint8_t)(address >> 8); + cal_checksum += (uint8_t)address; + bytes_read += 4; + count -= 2; + break; + + case 2: + /* S2 - 24 bit address data record */ + sscanf(&lpszLine[bytes_read], "%6" SCNx32, &address); + cal_checksum += (uint8_t)(address >> 16); + cal_checksum += (uint8_t)(address >> 8); + cal_checksum += (uint8_t)address; + bytes_read += 6; + count -= 3; + break; + + case 3: + /* S3 - 32 bit address data record */ + sscanf(&lpszLine[bytes_read], "%8" SCNx32, &address); + cal_checksum += (uint8_t)(address >> 24); + cal_checksum += (uint8_t)(address >> 16); + cal_checksum += (uint8_t)(address >> 8); + cal_checksum += (uint8_t)address; + bytes_read += 8; + count -= 4; + break; - while (count-- > 0) { - unsigned value; - sscanf(&lpszLine[bytes_read], "%2x", &value); - mot->buffer[cooked_bytes] = (uint8_t)value; - cal_checksum += (uint8_t)mot->buffer[cooked_bytes]; - bytes_read += 2; - cooked_bytes += 1; - section[image->num_sections].size += 1; - full_address++; - } - } else if (record_type == 5) { - /* S5 is the data count record, we ignore it */ - uint32_t dummy; - - while (count-- > 0) { - sscanf(&lpszLine[bytes_read], "%2" SCNx32, &dummy); - cal_checksum += (uint8_t)dummy; - bytes_read += 2; - } - } else if (record_type >= 7 && record_type <= 9) { - /* S7, S8, S9 - ending records for 32, 24 and 16bit */ - image->num_sections++; + } - /* copy section information */ - image->sections = malloc(sizeof(struct imagesection) * image->num_sections); - for (i = 0; i < image->num_sections; i++) { - image->sections[i].private = section[i].private; - image->sections[i].base_address = section[i].base_address; - image->sections[i].size = section[i].size; - image->sections[i].flags = section[i].flags; + if (full_address != address) { + /* we encountered a nonconsecutive location, create a new section, + * unless the current section has zero size, in which case this specifies + * the current section's base address + */ + if (section[image->num_sections].size != 0) { + image->num_sections++; + section[image->num_sections].size = 0x0; + section[image->num_sections].flags = 0; + section[image->num_sections].private = + &mot->buffer[cooked_bytes]; + } + section[image->num_sections].base_address = address; + full_address = address; + } + + while (count-- > 0) { + unsigned value; + sscanf(&lpszLine[bytes_read], "%2x", &value); + mot->buffer[cooked_bytes] = (uint8_t)value; + cal_checksum += (uint8_t)mot->buffer[cooked_bytes]; + bytes_read += 2; + cooked_bytes += 1; + section[image->num_sections].size += 1; + full_address++; + } + } else if (record_type == 5) { + /* S5 is the data count record, we ignore it */ + uint32_t dummy; + + while (count-- > 0) { + sscanf(&lpszLine[bytes_read], "%2" SCNx32, &dummy); + cal_checksum += (uint8_t)dummy; + bytes_read += 2; + } + } else if (record_type >= 7 && record_type <= 9) { + /* S7, S8, S9 - ending records for 32, 24 and 16bit */ + image->num_sections++; + + /* copy section information */ + image->sections = malloc(sizeof(struct imagesection) * image->num_sections); + for (i = 0; i < image->num_sections; i++) { + image->sections[i].private = section[i].private; + image->sections[i].base_address = section[i].base_address; + image->sections[i].size = section[i].size; + image->sections[i].flags = section[i].flags; + } + + end_rec = true; + break; + } else { + LOG_ERROR("unhandled S19 record type: %i", (int)(record_type)); + return ERROR_IMAGE_FORMAT_ERROR; } - return ERROR_OK; - } else { - LOG_ERROR("unhandled S19 record type: %i", (int)(record_type)); - return ERROR_IMAGE_FORMAT_ERROR; - } + /* account for checksum, will always be 0xFF */ + sscanf(&lpszLine[bytes_read], "%2" SCNx32, &checksum); + cal_checksum += (uint8_t)checksum; - /* account for checksum, will always be 0xFF */ - sscanf(&lpszLine[bytes_read], "%2" SCNx32, &checksum); - cal_checksum += (uint8_t)checksum; + if (cal_checksum != 0xFF) { + /* checksum failed */ + LOG_ERROR("incorrect record checksum found in S19 file"); + return ERROR_IMAGE_CHECKSUM; + } - if (cal_checksum != 0xFF) { - /* checksum failed */ - LOG_ERROR("incorrect record checksum found in S19 file"); - return ERROR_IMAGE_CHECKSUM; + if (end_rec) { + end_rec = false; + LOG_WARNING("continuing after end-of-file record: %.40s", lpszLine); + } } } - LOG_ERROR("premature end of S19 file, no end-of-file record found"); - return ERROR_IMAGE_FORMAT_ERROR; + if (end_rec) + return ERROR_OK; + else { + LOG_ERROR("premature end of S19 file, no matching end-of-file record found"); + return ERROR_IMAGE_FORMAT_ERROR; + } } /**