Hung-Te Lin (hungte@chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/2330
-gerrit
commit 248125b58607192f493eb1c4ce3ccb968d029f73 Author: Hung-Te Lin hungte@chromium.org Date: Sat Feb 9 10:38:55 2013 +0800
cbfstool: Fix crash on image without bootblock in end of ROM.
On platforms with CBFS data filling end of ROM image without bootblock in the end (ex, ARM), calculation of "next valid entry" may exceed ROM image buffer in memory and raise segmentation fault when we try to compare its magic value.
To fix this, always check if the entry address is inside ROM image buffer.
Verified to build and boot successfully on qemu/x86 and armv7/snow.
Change-Id: I117d6767a5403be636eea2b23be1dcf2e1c88839 Signed-off-by: Hung-Te Lin hungte@chromium.org --- util/cbfstool/cbfs_image.c | 26 +++++++++++++++----------- util/cbfstool/cbfs_image.h | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 5c2ac26..2cdbf23 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -112,7 +112,7 @@ static int cbfs_fix_legacy_size(struct cbfs_image *image) {
struct cbfs_file *entry, *first = NULL, *last = NULL; for (first = entry = cbfs_find_first_entry(image); - entry && cbfs_is_valid_entry(entry); + entry && cbfs_is_valid_entry(image, entry); entry = cbfs_find_next_entry(image, entry)) { last = entry; } @@ -352,7 +352,7 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer, cbfs_walk(image, cbfs_merge_empty_entry, NULL);
for (entry = cbfs_find_first_entry(image); - entry && cbfs_is_valid_entry(entry); + entry && cbfs_is_valid_entry(image, entry); entry = cbfs_find_next_entry(image, entry)) {
entry_type = ntohl(entry->type); @@ -429,7 +429,7 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer, struct cbfs_file *cbfs_get_entry(struct cbfs_image *image, const char *name) { struct cbfs_file *entry; for (entry = cbfs_find_first_entry(image); - entry && cbfs_is_valid_entry(entry); + entry && cbfs_is_valid_entry(image, entry); entry = cbfs_find_next_entry(image, entry)) { if (strcasecmp(CBFS_NAME(entry), name) == 0) { DEBUG("cbfs_get_entry: found %s\n", name); @@ -574,7 +574,7 @@ int cbfs_print_entry_info(struct cbfs_image *image, struct cbfs_file *entry, struct cbfs_payload_segment *payload; FILE *fp = (FILE *)arg;
- if (!cbfs_is_valid_entry(entry)) { + if (!cbfs_is_valid_entry(image, entry)) { ERROR("cbfs_print_entry_info: Invalid entry at 0x%x\n", cbfs_get_entry_addr(image, entry)); return -1; @@ -643,7 +643,7 @@ int cbfs_merge_empty_entry(struct cbfs_image *image, struct cbfs_file *entry,
next = cbfs_find_next_entry(image, entry);
- while (next && cbfs_is_valid_entry(next)) { + while (next && cbfs_is_valid_entry(image, next)) { type = ntohl(next->type); if (type == CBFS_COMPONENT_DELETED) { type = CBFS_COMPONENT_NULL; @@ -675,7 +675,7 @@ int cbfs_walk(struct cbfs_image *image, cbfs_entry_callback callback, int count = 0; struct cbfs_file *entry; for (entry = cbfs_find_first_entry(image); - entry && cbfs_is_valid_entry(entry); + entry && cbfs_is_valid_entry(image, entry); entry = cbfs_find_next_entry(image, entry)) { count ++; if (callback(image, entry, arg) != 0) @@ -730,7 +730,7 @@ struct cbfs_file *cbfs_find_next_entry(struct cbfs_image *image, struct cbfs_file *entry) { uint32_t addr = cbfs_get_entry_addr(image, entry); int align = ntohl(image->header->align); - assert(entry && cbfs_is_valid_entry(entry)); + assert(entry && cbfs_is_valid_entry(image, entry)); addr += ntohl(entry->offset) + ntohl(entry->len); addr = align_up(addr, align); return (struct cbfs_file *)(image->buffer.data + addr); @@ -741,9 +741,13 @@ uint32_t cbfs_get_entry_addr(struct cbfs_image *image, struct cbfs_file *entry) return (int32_t)((char *)entry - image->buffer.data); }
-int cbfs_is_valid_entry(struct cbfs_file *entry) { - return (entry &&memcmp(entry->magic, CBFS_FILE_MAGIC, - sizeof(entry->magic)) == 0); +int cbfs_is_valid_entry(struct cbfs_image *image, struct cbfs_file *entry) { + return (entry && + (char *)entry >= image->buffer.data && + (char *)entry + sizeof(entry->magic) < + image->buffer.data + image->buffer.size && + memcmp(entry->magic, CBFS_FILE_MAGIC, + sizeof(entry->magic)) == 0); }
int cbfs_init_entry(struct cbfs_file *entry, @@ -818,7 +822,7 @@ int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name, * by ELF loader and positioned by cbfs_add_entry. */ for (entry = cbfs_find_first_entry(image); - entry && cbfs_is_valid_entry(entry); + entry && cbfs_is_valid_entry(image, entry); entry = cbfs_find_next_entry(image, entry)) {
uint32_t type = ntohl(entry->type); diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 676efde..57bbfa1 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -114,7 +114,7 @@ struct cbfs_file *cbfs_find_next_entry(struct cbfs_image *image, uint32_t cbfs_get_entry_addr(struct cbfs_image *image, struct cbfs_file *entry);
/* Returns 1 if entry has valid data (by checking magic number), otherwise 0. */ -int cbfs_is_valid_entry(struct cbfs_file *entry); +int cbfs_is_valid_entry(struct cbfs_image *image, struct cbfs_file *entry);
/* Print CBFS component information. */ int cbfs_print_directory(struct cbfs_image *image);