futility: handle truncated BIOS images without segfault

A truncated BIOS with an otherwise valid FMAP that now points way
off the end of the file shouldn't cause coredumps.

BUG=none
BRANCH=ToT
TEST=make runtests

Change-Id: Idf96e1e6a381bf0fe0b1cb2d16e3dad39ce7a0dc
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/245500
Reviewed-by: Randall Spangler <rspangler@chromium.org>

6 files changed, 41 insertions, 8 deletions

diff --git a/futility/cmd_show.c b/futility/cmd_show.c
index c6584259..ca566c89 100644
--- a/futility/cmd_show.c
+++ b/futility/cmd_show.c
@@ -115,6 +115,13 @@ int futil_cb_show_gbb(struct futil_traverse_state_s *state)
 	int retval = 0;
 	uint32_t maxlen = 0;
 
+	if (!len) {
+		printf("GBB header:              %s <invalid>\n",
+		       state->component == CB_GBB ?
+		       state->in_filename : state->name);
+		return 1;
+	}
+
 	/* It looks like a GBB or we wouldn't be called. */
 	if (!futil_valid_gbb_header(gbb, len, &maxlen))
 		retval = 1;
@@ -234,6 +241,11 @@ int futil_cb_show_keyblock(struct futil_traverse_state_s *state)
  */
 int futil_cb_show_fw_main(struct futil_traverse_state_s *state)
 {
+	if (!state->my_area->len) {
+		printf("Firmware body:           %s <invalid>\n", state->name);
+		return 1;
+	}
+
 	printf("Firmware body:           %s\n", state->name);
 	printf("  Offset:                0x%08x\n", state->my_area->offset);
 	printf("  Size:                  0x%08x\n", state->my_area->len);
diff --git a/futility/misc.c b/futility/misc.c
index 183edcc2..24e645be 100644
--- a/futility/misc.c
+++ b/futility/misc.c
@@ -69,6 +69,9 @@ enum futil_file_type recognize_gbb(uint8_t *buf, uint32_t len)
 int futil_valid_gbb_header(GoogleBinaryBlockHeader *gbb, uint32_t len,
 			   uint32_t *maxlen_ptr)
 {
+	if (len < sizeof(GoogleBinaryBlockHeader))
+		return 0;
+
 	if (memcmp(gbb->signature, GBB_SIGNATURE, GBB_SIGNATURE_SIZE))
 		return 0;
 	if (gbb->major_version != GBB_MAJOR_VER)
diff --git a/futility/traversal.c b/futility/traversal.c
index aa9988a7..a661d9a5 100644
--- a/futility/traversal.c
+++ b/futility/traversal.c
@@ -133,7 +133,7 @@ enum futil_file_type recognize_bios_image(uint8_t *buf, uint32_t len)
 	return FILE_TYPE_UNKNOWN;
 }
 
-const char * const futil_cb_component_str[] = {
+static const char * const futil_cb_component_str[] = {
 	"CB_BEGIN_TRAVERSAL",
 	"CB_END_TRAVERSAL",
 	"CB_FMAP_GBB",
@@ -175,12 +175,22 @@ static int invoke_callback(struct futil_traverse_state_s *state,
 
 	if (cb_func[state->op][c])
 		return cb_func[state->op][c](state);
-	else
-		Debug("<no callback registered>\n");
 
 	return 0;
 }
 
+static void fmap_limit_area(FmapAreaHeader *ah, uint32_t len)
+{
+	uint32_t sum = ah->area_offset + ah->area_size;
+	if (sum < ah->area_size || sum > len) {
+		Debug("%s(%s) 0x%x + 0x%x > 0x%x\n",
+		      __func__, ah->area_name,
+		      ah->area_offset, ah->area_size, len);
+		ah->area_offset = 0;
+		ah->area_size = 0;
+	}
+}
+
 int futil_traverse(uint8_t *buf, uint32_t len,
 		   struct futil_traverse_state_s *state,
 		   enum futil_file_type type)
@@ -211,6 +221,8 @@ int futil_traverse(uint8_t *buf, uint32_t len,
 		for (area = bios_area; area->name; area++) {
 			/* We know this will work, too */
 			fmap_find_by_name(buf, len, fmap, area->name, &ah);
+			/* But the file might be truncated */
+			fmap_limit_area(ah, len);
 			retval |= invoke_callback(state,
 						  area->component,
 						  area->name,
@@ -227,6 +239,8 @@ int futil_traverse(uint8_t *buf, uint32_t len,
 		for (area = old_bios_area; area->name; area++) {
 			/* We know this will work, too */
 			fmap_find_by_name(buf, len, fmap, area->name, &ah);
+			/* But the file might be truncated */
+			fmap_limit_area(ah, len);
 			retval |= invoke_callback(state,
 						  area->component,
 						  area->name,
@@ -237,7 +251,13 @@ int futil_traverse(uint8_t *buf, uint32_t len,
 		}
 		break;
 
+	case FILE_TYPE_UNKNOWN:
+	case FILE_TYPE_CHROMIUMOS_DISK:
+		/* Nothing to do for these file types (yet) */
+		break;
+
 	default:
+		/* All other file types have their own callbacks */
 		retval |= invoke_callback(state,
 					  direct_callback[type].component,
 					  direct_callback[type].name,
diff --git a/futility/traversal.h b/futility/traversal.h
index e22b231e..47dd71bc 100644
--- a/futility/traversal.h
+++ b/futility/traversal.h
@@ -39,10 +39,6 @@ enum futil_cb_component {
 	NUM_CB_COMPONENTS
 };
 
-/* Names for them */
-extern const char * const futil_cb_component_str[];
-
-
 /* Where is the component we're poking at? */
 struct cb_area_s {
 	uint32_t offset;			/* to avoid pointer math */
diff --git a/host/lib/fmap.c b/host/lib/fmap.c
index 845d28f2..c95338db 100644
--- a/host/lib/fmap.c
+++ b/host/lib/fmap.c
@@ -65,4 +65,3 @@ uint8_t *fmap_find_by_name(uint8_t *ptr, size_t size, FmapHeader *fmap,
 
 	return NULL;
 }
-
diff --git a/tests/futility/test_dump_fmap.sh b/tests/futility/test_dump_fmap.sh
index 142a1540..bcdb27e6 100755
--- a/tests/futility/test_dump_fmap.sh
+++ b/tests/futility/test_dump_fmap.sh
@@ -24,6 +24,9 @@ cmp "${SCRIPTDIR}/data_fmap_expect_h.txt" "$TMP"
 # contain the stuff that the FMAP claims it does.
 if "$FUTILITY" dump_fmap -x "${SCRIPTDIR}/data_fmap.bin" FMAP; then false; fi
 
+# This should fail too
+if "$FUTILITY" show "${SCRIPTDIR}/data_fmap.bin"; then false; fi
+
 # However, this should work.
 "$FUTILITY" dump_fmap -x "${SCRIPTDIR}/data_fmap.bin" SI_DESC > "$TMP"
 cmp "${SCRIPTDIR}/data_fmap_expect_x.txt" "$TMP"